[External] : Re: OM World and Lilliput planning
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Jun 18 21:49:05 UTC 2024
Now I need a reviewer.
https://bugs.openjdk.org/browse/JDK-8334496
I don't have to mention UseObjectMonitorTable and UseHeavyMonitors
because they're going to be diagnostic flags.
thanks,
Coleen
On 6/18/24 11:59 AM, coleen.phillimore at oracle.com wrote:
>
> Hi Roman and Thomas, Thank you for your answers.
>
> On 6/18/24 5:43 AM, Kennke, Roman wrote:
>> The flag was never meant (by me) to be used by end-users. It’s been
>> good to have it available for testing and development, and it’s
>> served that purpose well. I’m happy to let it go.
>
> Great.
>>
>> I’m a bit undecided about keeping the legacy code in 25. On the one
>> hand, it feels safer to have the legacy as fallback for customers, in
>> case any troubles arise. While LW locking and also recursive LW
>> locking will be fairly well tested by then (at least by us), the
>> OMWorld stuff will probably still have a trail of problems then.
>> OTOH, keeping the legacy code around makes maintaining the locking
>> code messy and more difficult, and legacy might even have started to
>> bitrot by then (or not work at all, e.g. if Loom starts to rely on
>> LW/OMWorld stuff).
>
> If someone customer needs legacy locking going forward, it's because
> they have older code that suffers some large performance loss with
> lightweight locking, and more with the ObjectMonitor table (OM
> world). It might be severe enough to give them the switch to
> LM_LEGACY. We haven't gotten that sort of feedback but it doesn't
> mean it won't exist. For this reason, I think we should keep the
> Legacy code in JDK 25, since it's an LTS and such a customer may only
> upgrade to LTS releases, and then remove it in 26. Maintaining it
> won't be great in JDK 25, but the only backports we'd do from releases
> going forward would be bug fixes, so hopefully that code wouldn't bit
> rot badly there. It does complicate the code, a lot, especially with
> the UseObjectMonitorTable option.
>
> If Legacy is on, loom will pin synchronized code.
>
>>
>> I have similar feelings towards UseHeavyMonitors: on one hand it’s
>> nice to be able to turn off the LW locking altogether, but experience
>> shows that this has never really worked that well. For a long time,
>> it would only turn off stack-locking in some paths (e.g. interpreter)
>> but keep it in other (e.g. runtime), so it never really did what
>> people thought it would do. And if we don’t carefully maintain and
>> test this, it will bitrot, again. This is true whether or not we do
>> it diagnostic or develop, though. I’d probably vote for diagnostic
>> and then write some jtreg tests that verify that it does what it
>> says, or remove it altogether.
>
> It's a good idea to add some jtreg tests. There are 4 of them now,
> and I ran tier1 with it turned on, and had one expected failure. You
> and Thomas voted diagnostic, so I'll make it that, so it's available
> in production if we need to do some testing with it. Both diagnostic
> and develop flags can be removed anytime, so we could plan to remove
> it also in JDK 26. That would be nice.
>>
>> I also kinda agree with Stefan Karlsson about making OMWorld turned
>> on by default as soon as possible (maybe after giving it some short
>> bake-time off-by-default, to make sure the old stuff still works as
>> expected).
>
> The rationale for turning OMWorld off by UseObjectMonitorTable option
> is because I ran some performance tests, and there are results that
> might not be acceptable, and we need time to sort them out or find
> some way of mitigating the regressions. Our old friend, DaCapo xalan
> is a lot worse. Also Dacapo spring-large, Renaissance-ScalaKmeans
> (really bad). Apart from these the results aren't significantly worse,
> but performance testing always eats up a lot of time, which is why
> we'd want OMWorld off to start.
>
> Thanks,
> Coleen
>
>>
>> Cheers,
>> Roman
>>
>>
>>> On Jun 18, 2024, at 11:26 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>
>>> CAUTION: This email originated from outside of the organization. Do
>>> not click links or open attachments unless you can confirm the
>>> sender and know the content is safe.
>>>
>>> Hi Coleen,
>>>
>>> obsoleting the legacy mode feels a bit risky for an LTS. I am, of
>>> course, happy if the code can be removed.
>>>
>>> "and reintroduce (revert) the UseHeavyMonitors option as a develop
>>> flag, rather than a diagnostic flag"
>>>
>>> Why not diagnostic?
>>>
>>> Cheers, Thomas
>>>
>>>
>>> On Mon, Jun 17, 2024 at 3:21 PM <coleen.phillimore at oracle.com> wrote:
>>>
>>> Hi Roman, Thomas and Aleksey,
>>>
>>> I filed an RFE for the first part of this.
>>> https://bugs.openjdk.org/browse/JDK-8334299 and will be working on a
>>> CSR. I feel like we're backtracking a bit from what we did in JDK
>>> 22/23
>>> but this seems better to me. Comments?
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 6/5/24 3:19 AM, Stefan Karlsson wrote:
>>>> Hi Coleen,
>>>>
>>>> Thanks for moving the "OM World" towards completion. I have one
>>>> comment below:
>>>>
>>>> On 2024-06-04 22:52, coleen.phillimore at oracle.com wrote:
>>>>> Hi, This is what I wrote up after an internal discussion. I am about
>>>>> to file some RFEs/CSRs (or maybe will next week). Let me know what
>>>>> you think.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>> What we call OM World is saving the ObjectMonitor in a
>>>>> ConcurrentHashTable rather than in the markWord of the Java Object.
>>>>> Lilliput absolutely requires this since for Lilliput the Klass
>>>>> pointer is also in the markWord and to get to the Klass pointer for a
>>>>> locked object, the code would have to go to the displaced header in
>>>>> unboundedly racy situations.
>>>>>
>>>>> Without Lilliput, this is also helpful in that it frees up markWord
>>>>> bits for concurrent GCs or Valhalla to use. Because of this, and
>>>>> because of the high level of testing this type of change requires,
>>>>> we'd like to push this change to mainline ahead of the Lilliput work.
>>>>>
>>>>> OM World is built on top of Lightweight locking as Lightweight
>>>>> locking is required (doesn't save the stack location in the markWord
>>>>> as does Legacy locking). To reduce the maintenance burden and
>>>>> potential tricky interactions between new features and Legacy
>>>>> locking, we'd like to deprecate Legacy locking in JDK 24.
>>>>>
>>>>> Deprecating Legacy locking then makes the flag LockingMode not make
>>>>> any sense, as one of three enumerations will be missing. Also, to
>>>>> introduce OM World on top of Lightweight locking, it would be good to
>>>>> have that on a diagnostic flag in case of customer performance
>>>>> issues. It doesn't make sense to have a new locking mode for OM
>>>>> World, since it shares 80% code with Lightweight locking.
>>>>>
>>>>> Therefore I (with input from Axel and Stefan) propose the following
>>>>> for JDK 24:
>>>>>
>>>>> 1. Reintroduce the flag UseHeavyMonitors for LockingMode=LM_MONITOR
>>>>> 2. Deprecate LockingMode=LM_LEGACY
>>>>> 3. Deprecate the flag LockingMode. It's a new flag, legacy code
>>>>> won't miss it.
>>>>> 4. When OM World is ready to integrate, introduce a new diagnostic
>>>>> flag UseObjectMonitorTable
>>>>> - Start default off
>>>>> - Make it default on midway through JDK 24 if no problems.
>>>> What is the benefit of starting with this turned off and then a few
>>>> weeks later making it default? I think we'll get better functional
>>>> test coverage if it is enabled by default. We had a very similar
>>>> situation when Lightweight locking was turned off by default and many
>>>> bugs weren't found until it was turned on by default.
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>> JDK 25:
>>>>>
>>>>> 1. Obsolete Legacy locking mode (removes the code - TBD)
>>>>> 2. Obsolete LockingMode flag
>>>>> 3. We can hold onto UseObjectMonitorTable for a while (off turns off
>>>>> Lilliput UseCompactObjectHeaders).
>>>>>
>>
>>
>>
>> Amazon Web Services Development Center Germany GmbH
>> Krausenstr. 38
>> 10117 Berlin
>> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
>> Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
>> Sitz: Berlin
>> Ust-ID: DE 365 538 597
>
More information about the lilliput-dev
mailing list