qlibs/reflect

results compiling below code with -ftime-trace

Closed this issue · 15 comments

Continuing from our reddit discussion, I am attaching promised results
This is test code the same for both
obraz

obraz

magic_enum_reflect_ut.cc.json
reflect_enum_reflect_ut.cc.json
Screenshot_20240228_220426

You have 8 EvaluateAsRValue with lower and higher times
example 100ms wasted for unit test
EvaluateAsRValue reflect-src/reflect:337:19, col:76 96,178 ms
static_assert(std::string_view{"_42"} == enum_name<42, 256>(sparse::_42));

Thanks. Fixed. Please compare v1.0.8+. The following are the results for v1.0.8 with static_assert enabled.

Note: It's hard to compare exactly, as there are different features provided and different defaults.

time g++ -std=c++20 magic_enum.cpp # 0.322ms
time g++ -std=c++20 reflect.cpp # 0.318ms // with tests
time g++ -std=c++20 reflect.cpp # 0.187ms // without tests (`#define static_assert(...)`)

Tested code

magic_enum:HEAD

#include <magic_enum/magic_enum_all.hpp>

enum E { _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _16 };

int main() {
  (void)magic_enum::enum_name(E::_1);
  (void)magic_enum::enum_name(E::_2);
  (void)magic_enum::enum_name(E::_3);
  (void)magic_enum::enum_name(E::_4);
  (void)magic_enum::enum_name(E::_5);
  (void)magic_enum::enum_name(E::_6);
  (void)magic_enum::enum_name(E::_7);
  (void)magic_enum::enum_name(E::_8);
  (void)magic_enum::enum_name(E::_9);
  (void)magic_enum::enum_name(E::_10);
  (void)magic_enum::enum_name(E::_11);
  (void)magic_enum::enum_name(E::_12);
  (void)magic_enum::enum_name(E::_13);
  (void)magic_enum::enum_name(E::_14);
  (void)magic_enum::enum_name(E::_15);
  (void)magic_enum::enum_name(E::_16);
}

reflect:HEAD (v1.0.8)

#include <refelct>

enum E { _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _16 };

int main() {
  (void)magic_enum::enum_name(E::_1);
  (void)magic_enum::enum_name(E::_2);
  (void)magic_enum::enum_name(E::_3);
  (void)magic_enum::enum_name(E::_4);
  (void)magic_enum::enum_name(E::_5);
  (void)magic_enum::enum_name(E::_6);
  (void)magic_enum::enum_name(E::_7);
  (void)magic_enum::enum_name(E::_8);
  (void)magic_enum::enum_name(E::_9);
  (void)magic_enum::enum_name(E::_10);
  (void)magic_enum::enum_name(E::_11);
  (void)magic_enum::enum_name(E::_12);
  (void)magic_enum::enum_name(E::_13);
  (void)magic_enum::enum_name(E::_14);
  (void)magic_enum::enum_name(E::_15);
  (void)magic_enum::enum_name(E::_16);
}

Relevant changes:

??
Wall Duration 22,783 ms
"reflect::v1_0_8::(anonymous class)::operator()<reflect::v1_0_8::(lambda at /home/artur/projects/amrpc/targeo_amrpc/build/clang-release-time-trace-auto/_deps/reflect-src/reflect:569:33){}>"
static_assert(([]<auto expect = [](const bool cond) { return std::array{true}[not cond]; }> {

Wall Duration 68,613 ms
"reflect::v1_0_8::(anonymous class)::operator()<reflect::v1_0_8::(lambda at /home/artur/projects/amrpc/targeo_amrpc/build/clang-release-time-trace-auto/_deps/reflect-src/reflect:659:33){}>"

static_assert(([]<auto expect = [](const bool cond) { return std::array{true}[not cond]; }> {

Wall Duration 67,197 ms
"reflect::v1_0_8::(anonymous class)::operator()<reflect::v1_0_8::(lambda at /home/artur/projects/amrpc/targeo_amrpc/build/clang-release-time-trace-auto/_deps/reflect-src/reflect:746:33){}>"

static_assert(([]<auto expect = [](const bool cond) { return std::array{true}[not cond]; }> {

and few more ...

Can you elaborate, pleas. I don't follow this numbers as they are out of context. Is there any project/benchmark I can try or are there any steps I can follow to reproduce it, what setup is used, compiler, how many TUs? I don't see any issues from v1.0.8 with compilations times on benchmarks I'm performing, if anything, it seems faster than other projects, so would appreciate some guidance here so that I can track down what's happening.

Will continue later after work, can not right now.

no worries, thanks 👍

  1. example for reflect in this comments You wrote is invalid (magic_enum:: namespace)
  2. reflect doesn't work with clang and enums with undefined underlying type
    obraz
  3. Here You have comparison for reflect with unit tests on top compared to reflect with disabled unit tests for the example You provided in this topic but amended to enum E : uint8_t for clang
    main_cc
    And now explanation:
    For me it doesn't matter how it compares to magic_enum as magic_enum at production I can use only with a rule single enum_name wrapper in single translation unit per enum. When i tried when i started using it use it as constexpr scattering magic_enum::enum_name across project with ~1mln lines of code, it was disaster compiler was spending almost half of total time of building entire solution on compiling enum name from magic_enum.
    So now I am very cautious to compile time meta and check it carefully.
    For me it is a snow ball effect when in many files given include is included in project and used, unit tests of external project are simply not allowed at every solution compilation as it is pure waste of time, I don not unit test spdllog, ut, libc++ and many more at compile time, I can't ;-)
    I have ready pull request and will do it it is up to You what You are going to do with it.
    Btw amazing! work You did with ut and reflect.
    Pozdro.

Okay, thanks for the update, the details and traces. I understand the scale of the project and your requirements. I think it's fair to have option to disable the static tests overall. The only part important for my is that by default the tests will be enabled but can be opt out. Going to merge #37 but will refactor it slightly as a follow-up to avoid #ifdefs all over the place. Thanks.

Just to let You know
reflect:enum_name is 10x faster now in Function Instantiation than magic_enum, excellent work with reflect!
When I measured it with time-trace it is on my HW
magic_enum : 85,9 ms
reflect : 6,9 ms
obraz

Pretty cool, thanks for sharing. Just out of curiosity, it seems like you are compiling quite a big project with a lot translation units, hence was wondering whether you are using precompiled headers for third-party library or that not possible for some reason?

I am working on few projects in company so situation is complicated :-)
The better granulation of source code for multiple cores (many libraries and translation units) the less efficient pch is as You have bottle neck for pch creation at every library. So one of projects that is mostly my baby very well layered into multiple libraries with relatively small translation units, with high quality include hygiene is very fast for compiling on multiple cores but it doesn't like heavy includes, and using pch on it causes actually slower compile times that without pch. And I was hit not by inclusion times with magic_enum but with instantiation as you see 80ms per every enum_name in given enum type in given translation unit is a rather heavy influence on total time when used all across project and pch will not solve instantiation.
Second project result of work of multiple ppl has terrible translation units with even 30-50k lines of code and it is using pch.
I personally don't use pch in debug mode with IDE as it is affecting proper header hygiene during development.

Just to validate results, I looked at Your code and I am not sure if comparison is fair, as You cut names at runtime so reflect and magic enum have different outcome, ME generates enum literals while reflect generates functions names and runtime code so this is not very comparable by looking only just at Instantiation time.

Fixed now by 0f80f4f.
Expecting a bit slower compilation times as more things are done at run-time but not by much.

In real world case using enum_name without enum bounds it doesn't compile with clang
You declared reflect to work with clang so that's why i report

it's a relatively new constexpr check in clang since clang-16:
clang-15 just works https://godbolt.org/z/rPas4sT1z
it also works with with clang-16+ with -Wno-enum-constexpr-conversion - https://godbolt.org/z/n8Kcdzr6W. That can be done on the library side but I wanna avoid #pragma ignored in the code so looking for other options and don't wanna do it at run-time.