Introduce ENFORCE_FAST to speed up tests in debug builds #3341
Conversation
… timer code.
|
I am open to renaming the new ENFORCE directive, too (ENFORCE_NO_TIMER?). |
common/common.h
Outdated
| @@ -32,22 +32,32 @@ template <class E> using UnorderedSet = absl::flat_hash_set<E>; | |||
|
|
|||
| #define _MAYBE_ADD_COMMA(...) , ##__VA_ARGS__ | |||
|
|
|||
| // A faster version of ENFORCE that does not emit a timer. Useful for checks that happen extremely frequently and | |||
| // are O(1). Please avoid using unless ENFORCE shows up in profiles. | |||
| #define ENFORCE_FAST(x, ...) \ | |||
There was a problem hiding this comment.
We encourage people to prefer the fast thing many times in Sorbet (e.g., people cargo cult fast_sort).
I worry that people will think ENFORCE_FAST should be their default choice if they don't read the comment. What do you think about UNTIMED_ENFORCE or ENFORCE_WITHOUT_TIMER?
There was a problem hiding this comment.
ah just saw your comment (#3341 (comment)). That name works for me too!
There was a problem hiding this comment.
lgtm on concept and implementation (thanks for implementing one in terms of the other). i have qualm about naming, because this shows up a lot, and in common files that people are like to read while being new to the team, so i'd like to make sure we choose a name that encourages people to still prefer ENFORCE
Introduce ENFORCE_FAST to speed up tests in debug builds.
ENFORCE_FAST is equal to ENFORCE, except it doesn't create timers.
Motivation
Timers were making debug test runs >50% slower locally. With these changes, test/testdata/lsp/definitions_and_usages takes less than half as much time as it used to (7s-8s to ~3.3s on my laptop).
Timers still show up prominently in profiles, and I know there's probably another ~500ms of savings to trim, but this is good enough for now and will speed up my workflow.
Test plan
This change does not impact correctness.