On Fri, 25 Oct 2024 10:45:00 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
> > In that sense, I think we don't have a risk of potentially with > > `used_Integer > IntegerCache.high`. The idea described in Ioi's comment > > above (also brought up by Aleksey Shipilev separately during premain > > meeting) could be sufficient. > > Phew, thanks! I thought I was misunderstanding some fundamental thing here :) > I think `IntegerCache` interaction with CDS archive deserves a fix > regardless. Have you filed the bug for it, @jianglizhou? AFAICS, if we fix > `IntegerCache` <-> CDS interaction, we solve this particular problem as well. I filed https://bugs.openjdk.org/browse/JDK-8343019. I agree with what John said during the meeting yesterday, let's move away from cached Integer in this case (ModuleLoaderMap). > > I am still non-committal about this special fix. We can still do it, but then > this patch effectively changes relying on boxing identity behavior over > `Integer`s to relying on interning behavior over `Strings`, right? If we want > to be 100% safe, shouldn't `==` checks be rewritten to `equals`? And when we > do so, would that affect startup in any meaningful way? Right, it's based on archived interned String behavior. We don't disable any of the archived interned Strings at runtime. So I think we are indeed safe. In an offline chat with @cushon yesterday, Liam also asked about changing `==`. My sense was that there were probably performance considerations when the change was originally made and `==` was chosen (instead of `.equals()`) for performance reason. I think the performance difference mostly would be within noise level. Seems okay to change `.equals()`. I can do some measurements to compare. > > The names of the fields should probably be changed from `_INDEX` to something > else, like `_NAME`? Ok. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21672#issuecomment-2438567642
