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

Investigate potential performance improvements to api.h

    • Storage Engines
    • 8
    • 2024-02-20_A_near-death_puffin, 2024-03-05 - Claronald, 2024-03-19 - PacificOcean

      This ticket should assess changes to api.h, and make ones that improve performance.

      We can look at using __builtin_expect and a small refactoring of a couple of if statements in API_SESSION_INIT that can be combined.  My own findings (on macbook M1 w/ clang) showed that the __builtin_expect changes had performance regressions, and the if statement changes was a win - see the first comment on this ticket.  This should be checked on x86 and graviton, they are the platforms we care about.

       

      Here is the original Description for this ticket:

      Gcc and clang have a directive "__builtin_expect".  This rearranges compiled code so that the likely branch will have slightly better performance and in exchange the unlikely branch will have significantly worse performance.

      In a lot of cases, this can be tricky to use, the gcc doc says:

      In general, you should prefer to use actual profile feedback for this (-fprofile-arcs), as programmers are notoriously bad at predicting how their programs actually perform. 

      This is already used in places in error.h.  We have macros LIKELY and UNLIKELY defined in that file (should they be WT_LIKELY and WT_UNLIKELY?)

      I think it can make sense to use the UNLIKELY/WT_UNLIKELY macro in a case where we know that one case is not only unlikely, but one where we don't care much about performance in that case, for example, like an error path, or debugging path (that's usually off).

      I think we should consider (called from API_SESSION_INIT):

      #define WT_TRACK_OP_INIT(s)                                             \
          if (WT_UNLIKELY(F_ISSET(S2C(s), WT_CONN_OPTRACK)) && (s)->id != 0) {\
          .... 

      and there is a similar one in WT_TRACK_OP_END.

      In verbose.h (as __wt_verbose is called from API_SESSION_INIT):

      #define __wt_verbose_level(session, category, level, fmt, ...)           
          do {                                                                 
              if (WT_UNLIKELY(WT_VERBOSE_LEVEL_ISSET(session, category, level))
              ....
          } while (0)
       

      While we're at it, In API_SESSION_INIT we can refactor these lines to be:

        if ((s)->api_call_counter == 1) {                    \
              if (!F_ISSET(s, WT_SESSION_INTERNAL))          \
              __wt_op_timer_start(s);                        \
             (s)->cache_wait_us = 0;                         \
        }                                                    
      

      (I don't think the compiler can make this optimization on its own as it may believe that the value of session->api_call_counter can change (it can't).

      For this one in API_END

              if (ret != 0)                         \
                  __wt_txn_err_set(s, ret);         \
       

      I'd be wary about using WT_UNLIKELY, as the case includes WT_NOTFOUND, which seems reasonably likely and we don't want to run slower.  Perhaps (if the above LIKELY changes show measurable improvements), we can tackle this within the __wt_txn_err_set inline function.

            Assignee:
            will.korteland@mongodb.com Will Korteland
            Reporter:
            donald.anderson@mongodb.com Donald Anderson
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: