RFR: 8310264: In PhaseChaitin::Split defs and phis are leaked [v3]

[Johan Sjölen]

On Tue, 20 Jun 2023 10:23:29 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi,
>> 
>> `defs` and `phis` are leaked as they are resource allocated but not protected by a `ResourceMark`. The intention might have been for these to also live in the `split_arena`.. This change is the most conservative one, however, and does fix the memory leak.
>> 
>> Please consider, thanks.
>> 
>> Johan
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move these defs

Hi,

In pursuit of answering your questions I saw some more opportunities for improvement. I did end up removing the `ResourceMark` as the size of `Split` makes it difficult to see whether it changes something that's used later on or not.

First of all, the `VectorSet`s allocated their elements on the arena, but themselves on the resource area, this was another unhandled memory leak. I fixed that by allocating the VSets themselves on the arena, also. This removes a leak of `spill_cnt*56` bytes.

Second of all @shipilev asked a good question, and I think that we can move `defs` and `phis` into the arena. I was worried about the sizes of these arrays being quite large and thus triggering a lot of reallocations, but they're typically quite small (I measured) while sometimes reaching to defs being 256 and phis 512 elements. I picked a conservative lower bound of 8/16.

For what it's worth, the number of `VectorSet`s seems to strongly correlate with the size of the sets. a future RFE might make these larger by default. All of these measurements are done by running a Spring Hello World-app as a substitute for a 'real'/'typical' workload.

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

PR Comment: https://git.openjdk.org/jdk/pull/14530#issuecomment-1598520575