[9] RFR(S): 8136458: Remove "marked for reclamation" nmethod state

[Tobias Hartmann]

Hi Nils,

thanks for looking at this!

On 18.03.2016 13:49, Nils Eliasson wrote:
> Was it the move to a dedicated sweeper thread that make this possible? When the compiler threads swept the code cache, it was done in parts, and then we had to keep the nmethods as marked-for-reclaation until a full sweep was completed. Am I right? Then this makes perfect sense.

No, the dedicated sweeper thread did not change this behavior. Before 8046809 [1], sweeping was done by the compiler threads in several steps to reduce pressure and establish a good balance between sweeping and compiling. Now having a dedicated thread (that may be interrupted at any time), does not affect the nmethod state cycle but allows the thread scheduler to find the best balance between compilation and sweeping. We still have to wait for a full sweep to finish before a zombie nmethod is flushed. My point is that we do this anyway and don't need the "marked for reclamation" state for this.

Basically, my assumptions are the following:
After a nmethod was marked as zombie by the sweeper, it's guaranteed to be not on the stack and no inline caches will be updated to point to this nmethod. However, outdated ICs may still point to it. The sweeper will continue to visit nmethods and only encounter the zombie nmethod again, after a full sweep cycle is finished. This is guaranteed because nmethods are not moved in the code cache. Therefore, all the inline caches of other nmethods that pointer to the zombie nmethod are cleaned now. We can safely flush the nmethod.

I'm not sure why the "marked for reclamation" state was ever necessary (if at all).

Best regards,
Tobias

[1] http://cr.openjdk.java.net/~anoll/8046809/webrev.06/

> 
> Looks good,
> Nils
> 
> On 2016-03-18 10:22, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8136458
>> http://cr.openjdk.java.net/~thartmann/8136458/webrev.00/
>>
>> The sweeper removes zombie nmethods only after they were "marked for reclamation" to ensure that there are no inline caches referencing the zombie nmethod. However, this is not required because if a zombie nmethod is encountered again by the sweeper, all ICs pointing to it were already cleaned in the previous sweeper cycle:
>>
>> alive -> not-entrant/unloaded (may be on the stack)
>> cycle 1: not-entrant/unloaded -> zombie (may be referenced by ICs)
>> cycle 2: zombie -> marked for reclamation
>> cycle 3: marked for reclamation -> flush
>>
>> In each cycle, we clean all inline caches that point to not-entrant/unloaded/zombie nmethods. Therefore, we know already after sweeper cycle 2, that the zombie nmethod is not referenced by any ICs and we could flush it immediately.
>>
>> I removed the "marked for reclamation" state. The following testing revealed no problems:
>> - JPRT
>> - RBT with hotspot_all and -Xcomp/-Xmixed
>> - 100 iterations of Nashorn + Octane with -XX:StartAggressiveSweepingAt=100/50 -XX:NmethodSweepActivity=500/100
>>
>> Thanks,
>> Tobias
>