Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8237842

Separate definitions for default cache line and padding sizes

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Fixed
    • Icon: P4 P4
    • 23
    • 15
    • hotspot
    • None
    • gc
    • b05
    • generic
    • generic

      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.

            shade Aleksey Shipilev
            dcubed Daniel Daugherty
            Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

              Created:
              Updated:
              Resolved: