apple / foundationdb

FoundationDB - the open source, distributed, transactional key-value store
https://apple.github.io/foundationdb/
Apache License 2.0
14.19k stars 1.29k forks source link

ASSERTion in flow may be improved by bringing in less branches #11422

Closed xis19 closed 2 weeks ago

xis19 commented 1 month ago

ASSERT in FDB might be painful since 1. it requires an if branch, which might hurt the pipeline (https://stackoverflow.com/questions/3702903/portable-branch-prediction-hints) 2. It calls extern isAssertDisabled, which may or may not be inlined while the value seems true in most cases -- even inlined, causes lots of tests. A certain optimization could be:

  1. Use [[unlikely]] or [[likely]] to hint the compiler about the branching
  2. Set a variable for isAssertDisabled, rather than a function call.
xis19 commented 4 weeks ago

It is amusing to see that if ASSERT is shortcut, e.g.

#define ASSERT(condition) ((void)0)

then the simulation will fail.

xis19 commented 4 weeks ago

So some of the ASSERT conditions are stateful. Need to check them out.

xis19 commented 4 weeks ago

Tested a bit more about the performance. Using args

-r simulation -b off -f ../CycleTest.toml -s 1024

as a reference, seeing that the performance is not significantly changed with ASSERT turned on/off. This might make sense since the assert is defined as

#define ASSERT(condition)                                                                                              \
    do {                                                                                                               \
        if (!((condition) || isAssertDisabled(__LINE__))) {                                                            \
            throw internal_error_impl(#condition, __FILE__, __LINE__);                                                 \
        }                                                                                                              \
    } while (false)

and the condition is true in most cases, so the following function call is short-cut.