JDK 23 |
---|
23 b05Fixed |
Blocks :
|
|
Blocks :
|
|
Blocks :
|
|
Relates :
|
Claes posted this comment in response to a comment from David Holmes in the JDK-8235931 review: On 12/18/19 10:03 AM, Claes Redestad wrote: > Hi, > > On 2019-12-18 06:50, David Holmes wrote: >> Hi Dan, >> >> On 18/12/2019 7:34 am, Daniel D. Daugherty wrote: >>> Greetings, >>> >>> I'm extracting another standalone fix from the Async Monitor Deflation >>> project (JDK-8153224) and sending it out for review (and testing) >>> separately. This is an XXS sized fix, but since it involves cache >>> line sizes I don't think a trivial review is appropriate. >> >> Can you elaborate on why a smaller cache-line-size assumption is beneficial here? Do you have performance numbers for the bug report? > > I did numerous experiments on this during work on JEP-143 and saw no > improvement on neither "real" benchmarks nor stress tests that try to > explicitly provoke cross-monitor false sharing on various fields in > ObjectMonitors. I've not re-run those experiments on new hardware, so > YMMV. Should probably check if they're still usable.. > > Specifically, for padding values below 64 we could provoke false sharing > effects on stress tests, with negligible movement on "real" benchmarks. > For values above 64 we just waste space. > > There exists a legend that you need double the physical cache line > padding to not provoke the wrath of false sharing in the presence of > aggressive prefetching, which is the reason why the > "DEFAULT_CACHE_LINE_SIZE" is set to twice the typical physical cache > line size in bytes. If we explicitly think we need to pad two lines, > then the constant DEFAULT_CACHE_LINE_SIZE should be named differently. > > The patch in question looks good to me, although I'd prefer something > like: > > CACHE_LINE_SIZE 64 > DOUBLE_CACHE_LINE_SIZE 128 > > .. and then use whichever is deemed more appropriate on a case by case > basis. (Hint: it's probably CACHE_LINE_SIZE) > > $.02 > > /Claes The renaming of the existing DEFAULT_CACHE_LINE_SIZE CACHE_LINE_SIZE or DOUBLE_CACHE_LINE_SIZE is beyond the scope of what I was doing with JDK-8235931 so I'm filing this followup RFE. Since DEFAULT_CACHE_LINE_SIZE is used by several of the subsystems in HotSpot, it's not really clear where this bug should be filed. I've started with hotspot/runtime, but the triage team may move this elsewhere.
|