On Mon, 13 May 2024 14:08:01 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Then I would even remove the double-checking idiom, the `volatile` on `ctor` >> and `properties`, and declare methods `getProperties()` and >> `ensureConstructors()` as `synchronized`. >> I'm not sure that the double-checking optimization brings much value on >> contemporary JVMs. >> >> But I feel that the followup PR discussed before wouldn't need >> `synchronized` at all. >> >> WDYT? > >> Then I would even remove the double-checking idiom, the volatile on ctor and >> properties, and declare methods getProperties() and ensureConstructors() as >> synchronized. >> I'm not sure that the double-checking optimization brings much value on >> contemporary JVMs. > > Making the methods synchronized would bring in a penalty that there will > always be a monitor entry at every call site, even after the `properites` and > `ctor`(s) are initialized. Ideally, we should just do all of this > intialization in the constructor of the `RandomGeneratorFactory`, the one > which takes the `Class<>` type of the `RandomGenerator`. We can then make the > `properties` and the `ctor`(s) all `final` and not have to worry about any > synchronization or volatile semantics. You would of course have to rework the > ensureConstructors to not throw an exception at that time. > >> But I feel that the followup PR discussed before wouldn't need synchronized >> at all. > > Correct. The more I think about it, I think cleaning up all this in this PR > itself might make both reviewing and the implementation a bit more simpler. > What's your thoughts? OK, will do all the work in this PR. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1598548744
