Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-86602

Usability improvements to our wrapper around std::atomic

    • Type: Icon: Improvement Improvement
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • Labels:
      None

      • Rename from AtomicWord<T> to Atomic<T>
      • Make the conversion constructor from T implicit to support Atomic<int> answer = 42; syntax.

      Both changes were discussed and approved on slack.

      Should we rename AtomicWord<T> to just Atomic<T>? I don't think the word Word is adding anything except possible confusion. "Word" usually refers to either an arbitrary platform specific size or to the native register size. But in this case it means neither. It didn't matter much back in the old days when nobody used it directly and we used the typedefs like AtomicInt. But those are now gone. Should we simplify the template name now that it is the main API?

      On a related note, any objection to making the AtomicWord<T>::AtomicWord(T) constructor implicit like it is in std::atomic? It has long bothered me that it requires Atomic<int> blah{1}; rather than the more natural Atomic<int> blah = 0; initialization syntax. I think the usual arguments against implicit conversions don't apply here: , 1) Atomics aren't assignable from other atomics (we support neither, but std::atomic supports = T while not supporting = atomic<T> because that can't be done atomically) 2) The initialization is non-atomic anyway and non racy so the usual reason we want explicit load() and store() calls doesn't apply. I think the main difference is whether we support = in initialization. 3) This won't make any operations other than initialization implicit.
      The only other difference I can think of is that f(const Atomic<T>&) can now be called with a T. I'm not sure that is even wrong. Plus it is very rare. Grepping for const Atomic I only see 6 cases of that in our codebase, and it doesn't seem problematic for any of them. 2 already have overloads that would accept a T already, and the others have sentinel values that one could imagine wanting to pass directly. I suppose f(Atomic<T>) and f(Atomic<T>&&) are also technically possible, and would allow the conversion, but those are already nonsensical even without implicit conversions.
      Historically, that type predates C++11, so it wasn't an explicit choice to differ from std::atomic, more just defaulting to explicit for single-arg constructors was seen as a good idea. And since it was pre C++11, there was no in-class initialization or Type _var{val}; anyway, so most atomics would be initialized in the ctor like _val(1) so it didn't make any difference

            Assignee:
            mathias@mongodb.com Mathias Stearn
            Reporter:
            mathias@mongodb.com Mathias Stearn
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: