On Wed, 26 Mar 2025 16:25:07 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> What would you think about using the simple code + loop from the JBS >> comment? That would remove any discussion about toArray and any concerns >> that it would load classes. > > Regarding adding a comment, I wasn't too sure what the comment should say > because once you start down that path, it's hard to not end up with too much > detail, and then things just get very wordy. Since we have a test that will > fail if this is ever modified to cause classloading again, at least that > should help prevent undoing this fix. However, the one downside of the test > is that I can only get it to fail with non-debug builds, so it may not be > caught early. > > Regarding using the loop, that's probably a more bullet proof option, > although there is still no guarantee that someday someone won't look at it > and say "I can just use toArray() here". There's probably no escaping an AI > code cleaning bot coming to that conclusion sometime in the near future. Then > we're back to adding a comment to make sure it isn't changed. I just noticed that this `subgroupsAsArray()` private method is only used by JVMTI and a comment exists about that on that method: /** * Returns a snapshot of the subgroups as an array, used by JVMTI. */ So, irrespective of what solution we choose, perhaps we could add another brief sentence to that comment, something like the following? /** * Returns a snapshot of the subgroups as an array, used by JVMTI. * Care should be taken not to trigger any classloading in this method. */ ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24236#discussion_r2014580285
