RFR: 8329665: fatal error: memory leak: allocating without ResourceMark [v2]

[Patricio Chilano Mateo]

On Mon, 1 Jul 2024 13:05:36 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Yes, I was afraid of something like that. Please open a follow-up bug, and chain it to 8329665, since the fix has already been downported to JDK21. We should fix this before the October update. I added a little comment to 8329665 to ensure this does not get lost.
> 
Filed https://bugs.openjdk.org/browse/JDK-8335409.

> I was thinking along of this line.
> 
> The performance comment makes sense for lookup, but not so much here. Plus, I would wager this case is rare anyway since the InterpreterOopMap can hold what, (4*64)/2bit = 128 entries?
> 
Yes. I actually run tiers1-6 and found there are very few methods where we hit this code path where 128 entries are not enough so we need to allocate (~ less than 10). Most are tests to exercise this corner case. One of the only legit ones I found was TimeZoneNames_xx.java.getContents(). So it is a rare case.

> About the other comment (` // Due to the invariants above it's tricky to allocate a temporary OopMapCacheEntry on the stack`), that is weird. I don't get it. It is ages old (comes from initial commit), so you may be more able than I to dig out the history behind it. I tried to just allocate an InterpreterOopMap entry on the stack. I did a small test, nothing bad happened.
> 
> So, if this comment is not up to date, you may pay for the C-heap allocation by removing this allocation :)
> 
Sounds we should be able to allocate in the stack. I'll check this.
 
> > What do you think? I don't actually see a particular reason to disallow this allocations so I'm now not convinced on 1.
> 
> I prefer 2.
>
Okay, I was thinking the same. I'll work on this approach.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18632#issuecomment-2200217314