RFR: 8201516: DebugNonSafepoints generates incorrect information [v2]

Tobias Hartmann thartmann at openjdk.org
Thu Mar 2 09:49:12 UTC 2023


On Wed, 1 Mar 2023 17:51:14 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed default argument
>
> src/hotspot/share/opto/phaseX.cpp line 474:
> 
>> 472:   GrowableArray<Node_Notes*>* old_node_note_array = C->node_note_array();
>> 473:   if (old_node_note_array != NULL) {
>> 474:     C->set_node_note_array(new (C->comp_arena()) GrowableArray<Node_Notes*> (C->comp_arena(), 8, 0, NULL));
> 
> Use `nullptr` in these lines.
> Can you use `_useful.size()` as initial array length?

Thanks for the review, Vladimir. I updated the `NULL` usages.

> Can you use _useful.size() as initial array length?

The `node_note_array` uses buckets/blocks of size `C->_node_notes_block_size == 256`. So the actual required size would be `ceil((double)useful.size() / (double)256)` but we could simply use `1 + (useful.size() / 256)`. 

Now even when initializing the GrowableArray to that size, setting notes will then still call `Compile::grow_node_notes` multiple times to create the `Node_Notes` buckets and since it always at least doubles the backing array size, we actually end up with an array that is larger than what is required:
https://github.com/openjdk/jdk/blob/4619e8bae838abd1f243c2c65a538806d226b8e8/src/hotspot/share/opto/compile.cpp#L1233-L1238

I updated the code to properly pre-size the structure by calling `grow_node_notes`. This also has the advantage that the `Node_Notes` arena is one big chunk instead of incrementally allocating small ones.

What do you think?

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

PR: https://git.openjdk.org/jdk/pull/12806


More information about the hotspot-compiler-dev mailing list