RFR: 8306444: Don't leak memory in PhaseChaitin::PhaseChaitin
Vladimir Kozlov
kvn at openjdk.org
Wed Apr 19 17:54:49 UTC 2023
On Wed, 19 Apr 2023 13:12:00 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Hi,
>
> First, `PhaseChaitin::PhaseChaitin` used to create 4 resource array of size `_cfg.number_of_blocks`: one to store all of the block pointers in (`_blks`), and three to do a sorting of the blocks in some order. The latter three weren't freed in the constructor, causing them to hang around for the entire duration of the phase. This is unnecessary, so this patch frees the arrays when we're done with them. It also allocates all of the resources arrays in one go.
>
> Second, it copied over each partially filled bucket into the `_blks`, one block at a time. This patch changes this so that we don't allocate the `_blks` resource array at all, instead we simply squash all of the partially filled buckets into the first one using `::memmove`.
>
> I haven't done any micro benchmarking, but this should be faster and take less space.
>
> This is currently passing tier1.
Changes are good. Just few comments
src/hotspot/share/opto/chaitin.cpp line 250:
> 248: uint cnt = buckcnt[j];
> 249: // Assign block to end of list for appropriate bucket
> 250: buckets[j][cnt] = _cfg.get_block(i);
You can use `blk` here.
src/hotspot/share/opto/chaitin.cpp line 263:
> 261: ::memmove(offset, buckets[i], buckcnt[i]*sizeof(Block*));
> 262: offset += buckcnt[i];
> 263: }
May add assert that `assert((offset - &buckets[0][0]) == nr_blocks`
src/hotspot/share/opto/chaitin.cpp line 265:
> 263: }
> 264: // Free the now unused memory
> 265: FREE_RESOURCE_ARRAY(Block*, buckets[1], (NUMBUCKS-1)*nr_blocks);
I did not know that you can free part of allocated space.
-------------
PR Review: https://git.openjdk.org/jdk/pull/13533#pullrequestreview-1392572048
PR Review Comment: https://git.openjdk.org/jdk/pull/13533#discussion_r1171657776
PR Review Comment: https://git.openjdk.org/jdk/pull/13533#discussion_r1171680140
PR Review Comment: https://git.openjdk.org/jdk/pull/13533#discussion_r1171675541
More information about the hotspot-compiler-dev
mailing list