On Tue, 30 Aug 2022 11:52:24 GMT, Roman Kennke <rken...@openjdk.org> wrote:
>> I didn't realize you still also is using the frame basic lock area. (in >> other projects this is removed and all cases are handled via the threads >> lock stack) >> So essentially we have two lock stacks when running in interpreter the frame >> area and the LockStack. >> >> That explains why I have not heard anything about popframe and friends :) > >> I didn't realize you still also is using the frame basic lock area. (in >> other projects this is removed and all cases are handled via the threads >> lock stack) So essentially we have two lock stacks when running in >> interpreter the frame area and the LockStack. >> >> That explains why I have not heard anything about popframe and friends :) > > Hmm yeah, I also realized this recently :-D > I will have to clean this up before going further. And I'll also will work to > support the unstructured locking in the interpreter. > We need to understand performance effects of these changes. I don't see data > here or new JMH benchmarks which can show data. @rkennke can you show data > you have? And, please, update RFE description with what you have in PR > description. I did run macro benchmarks (SPECjvm, SPECjbb, renaissance, dacapo) and there performance is most often <1% from baseline, some better, some worse. However, I noticed that I made a mistake in my benchmark setup, and I have to re-run them again. So far it doesn't look like the results will be much different - only more reliable. Before I do proper re-runs, I first want to work on removing the interpreter lock-stack, and also to support 'weird' locking (see discussion above). I don't expect those to affect performance very much, because it will only change the interpreter paths. I haven't run any microbenchmarks, yet, but it may be useful. If you have any, please point me in the direction. > I would prefer to have "opt-in" but looking on scope of changes it may > introduce more issues. Without "opt-in" I want performance comparison of VMs > with different implementation instead of using `UseHeavyMonitors` to make > judgement about this implementation. `UseHeavyMonitors` (product flag) should > be tested separately to make sure when it is used as fallback mechanism by > customers they would not get significant performance penalty. Yes, I can do that. > I agree with @tstuefe that we should test this PR a lot (all tiers on all > supported platforms) including performance testing before integration. In > addition we need full testing of this implementation with `UseHeavyMonitors` > ON. Ok. I'd also suggest to run relevant (i.e. what relates to synchronized) jcstress tests. > And I should repeat that integration happens when changes are ready (no > issues). We should not rush for particular JDK release. Sure, I am not planning on rushing this. ;-) > I didn't realize you still also is using the frame basic lock area. (in other > projects this is removed and all cases are handled via the threads lock > stack) So essentially we have two lock stacks when running in interpreter the > frame area and the LockStack. > > That explains why I have not heard anything about popframe and friends :) I'm now wondering if what I kinda accidentally did there is not the sane thing to do. The 'real' lock-stack (the one that I added) holds all the (fast-)locked oops. The frame basic lock area also holds oops now (before it was oop-lock pairs), and in addition to the per-thread lock-stack it also holds the association frame->locks, which is useful when popping interpreter frames, so that we can exit all active locks easily. C1 and C2 don't need this, because 1. the monitor enter and exit there is always symmetric and 2. they have their own and more efficient ways to remove activations. How have you handled the interpreter lock-stack-area in your implementation? Is it worth to get rid of it and consolidate with the per-thread lock-stack? ------------- PR: https://git.openjdk.org/jdk/pull/9680
