Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-11749

When failing to compare and swap don't re-read the expected value.

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

      The atomic builtin __atomic_compare_exchange_n is used throughout WiredTiger although it is wrapped by WT_ATOMIC_CAS_FUNC. The typical pattern is:

          do {
              /* Read the expected value. */
              last_recno = btree->last_recno;
             /* Swap in the new value. */
          } !__wt_atomic_cas64(&btree->last_recno, last_recno, recno));
      

      This example was trimmed down from here.

      If we fail to CAS then we re-read the expected value. This is an slight misuse of the builtin function, the docs state:

      This built-in function implements an atomic compare and exchange operation. This compares the contents of *ptr with the contents of *expected. If equal, the operation is a read-modify-write operation that writes desired into *ptr. If they are not equal, the operation is a read and the current contents of *ptr are written into *expected. 
      

      By re-reading the expected value, the read inside the body of the do-while loop, we are adding an extra read of the value and not properly utilizing the functionality of the CAS builtin.

      This loop could be:

      while (!__wt_atomic_cas64(&btree->last_recno, last_recno, recno)) {}
      

      Additionally per the C standard, the second read creates a data race as the variable is being written to atomically and not read from atomically. Whereas if we utilized the read given to us by the failed CAS we would avoid this data race.

      Scope:

      • Discuss the implications of this and whether it should be implemented.
      • Update relevant usages or split off tickets if there are too many.

            Assignee:
            backlog-server-storage-engines [DO NOT USE] Backlog - Storage Engines Team
            Reporter:
            luke.pearson@mongodb.com Luke Pearson
            Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated: