RFR (S/M): 8012547: Reclamation of flushed methods can be slow

[Nils Eliasson]

Thanks, fixed.

//N

On 2013-04-26 21:08, Christian Thalinger wrote:
> On Apr 26, 2013, at 8:17 AM, Nils Eliasson <nils.eliasson at oracle.com> wrote:
>
>> On 2013-04-25 18:59, Christian Thalinger wrote:
>>> Since someone has to start with this:  could you please change the method comments you've added to Doxygen-style comments?
>>>
>>> -nmethod* CodeCache::find_and_remove_saved_code(Method* m) {
>>> +// Remove and return nmethod from the saved code list in order to reanimate it.
>>> +nmethod* CodeCache::reanimate_saved_code(Method* m) {
>>>
>>> to:
>>>
>>> /**
>>>   * Remove and return nmethod from the saved code list in order to reanimate it.
>>>   */
>>> nmethod* CodeCache::reanimate_saved_code(Method* m) {
>>>
>>> Otherwise this looks good.
>>>
>>> -- Chris
>> ok, added that.
> Thanks.  There are typos, though:
>
> + * Remove nmethod from the save code list in order to discard it permanentely
>
> Should be "saved code" and "permanently".
>
> -- Chris
>
>> New webrev including Kozlovs comments to:
>>
>> http://cr.openjdk.java.net/~neliasso/8012547/webrev.08/ <http://cr.openjdk.java.net/%7Eneliasso/8012547/webrev.08/>
>>
>> Thanks!
>> //Nils
>>
>>> On Apr 25, 2013, at 7:05 AM, Nils Eliasson <nils.eliasson at oracle.com> wrote:
>>>
>>>> On 2013-04-25 04:37, Christian Thalinger wrote:
>>>>> On Apr 24, 2013, at 7:16 AM, Nils Eliasson <nils.eliasson at oracle.com> wrote:
>>>>>
>>>>>> This is the correct webrev for the RFR I sent a moment ago:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~neliasso/8012547/webrev.04/ <http://cr.openjdk.java.net/%7Eneliasso/8012547/webrev.04/>
>>>>> src/share/vm/runtime/sweeper.cpp:
>>>>>
>>>>> +          (_traversals > _last_flush_traversal_id + 2) && ((uint)nm->compile_id() < _highest_marked)) {
>>>>>
>>>>> Can't we change _highest_marked to int?  I see that CompileBroker::get_compilation_id() returns an uint.  We should decide if compile ids are int or uint.  I vote for int.
>>>> Yes. They can only be positive, and 0 is used blobs without ID. But int is the type used in other places so I will change to that and make it a little bit more consistent.
>>>>
>>>>> I don't see _dead_compile_ids being ever set.
>>>> It isn't :P TortoiseHG reverts changes on qrefresh when it thinks the patch is public. Without notice.
>>>>
>>>> Fixed.
>>>>
>>>> New webrev including your and Vladimirs suggestions:
>>>> http://cr.openjdk.java.net/~neliasso/8012547/webrev.07 <http://cr.openjdk.java.net/%7Eneliasso/8012547/webrev.07>
>>>>
>>>> Thanks!
>>>> //Nils
>>>>
>>>>> -- Chris
>>>>>
>>>>>> //Nils
>>>>>>
>>>>>> On 2013-04-18 15:09, Nils Eliasson wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> I have another fix to the code cache sweeper and flushing that needs a review.
>>>>>>>
>>>>>>> The major change is to remove a check in scan_stacks that stops the sweeper when the cache is getting  full. The normal mode is to sweep and record if any change has happened that require another sweep. This check stops the sweeper early causing some methods that are speculatively disconnected to stay so for an unnecessary long time sometimes causing unnecessary new flushes.
>>>>>>>
>>>>>>> Also some refactoring
>>>>>>> - remove state variable _do_sweep that was unnecessary. It marked if a sweep was active or not, but just a duplicate way of checking if any methods are being sweept (nmethodsweeper::current != NULL).
>>>>>>> - rename _rescan to _resweep. When _resweep is set there will be another sweep started when the current ends. That sweep will start with a scan, but it is not only a scan.
>>>>>>> - rename _advise_to_sweep to _flush_token. Is CASed by the first thread that wants to flush and reset by scan_stacks when the flush is finished and a proper time has passed.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~neliasso/8012547/ <http://cr.openjdk.java.net/%7Eneliasso/8012547/>
>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8012547
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Nils E.