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

resolve atomic.hpp wording issues from JDK-8047104 code review

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: P4 P4
    • 9
    • 9
    • hotspot
    • None
    • b25
    • generic
    • generic
    • Not verified

        The following issue came up during the code review cycle for the following:

        JDK-8047104 cleanup misc issues prior to Contended Locking
                        reorder and cache line bucket

        On 6/20/14 9:23 AM, Daniel D. Daugherty wrote:
        >
        >> A few nits and queries and one more significant issue, which happens to be first:
        >>
        >> src/share/vm/runtime/atomic.hpp
        >>
        >> + // add* provide full bidirectional fence, i.e. storeload/read-modify-write/storeload
        >>
        >> That's not a "full bi-directional fence". A fence is: loadLoad|loadStore|storeLoad|storeStore - as per orderAccess.hpp. The cmpxchg wording:
        >>
        >> "Guarantees a two-way memory barrier across the cmpxchg. I.e., it's really a 'fence_cmpxchg_acquire'."
        >>
        >> was accurate in terms of required behaviour for cmpxchg. Whether the fence and acquire needed to be explicit depends on the platform.
        >>
        >> It is also not clear to me that other than cmpxchg* those atomic ops need "full bi-directional fences", or whether the current implementations actually provide them.
        >
        > This item will take some research to resolve so I'll have to reply
        > again on this one later.

        We've been trying to reach closure on the new wording for atomic.hpp
        in off-thread e-mails for almost two weeks. At this point, I'm dropping
        the atomic.hpp changes from this bug fix (8047104). Changes to the
        wording in atomic.hpp may happen in a future Contended Locking bucket,
        but for now it's not happening.

              dcubed Daniel Daugherty
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Created:
                Updated:
                Resolved: