Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc #4094

jleibs · 2023-10-30T22:19:19Z

What

Resolves: Logging SDK speed regression #4093

Arrow was default to a Debug build regardless of what CMAKE_BUILD_TYPE was specified.

Baseline:

$ time RERUN=on ./build/rerun_vrs_example sample.vrs
real    0m15.105s
user    0m16.409s
sys     0m2.981s

CMAKE_BUILD_TYPE=RelWithDebInfo

$ time RERUN=on ./build/rerun_vrs_example sample.vrs
real    0m4.659s
user    0m5.259s
sys     0m2.257s

-DARROW_MIMALLOC=ON

$ time RERUN=on ./build/rerun_vrs_example sample.vrs
real    0m3.477s
user    0m5.015s
sys     0m1.291s

Test

Linux
Mac
Windows, rerun build & test only

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested demo.rerun.io (if applicable)
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

rerun_cpp/CMakeLists.txt

Wumpf · 2023-10-31T07:52:47Z

Let's make sure this works on Mac & Windows before mergning
I remember disabling jemalloc because of build issues somewhere (don't know which one) and turning it off helped (didn't think much of perf impact at that point ofc :( ). Still opens the question though whether we should use jemalloc over mimalloc or not.

emilk · 2023-10-31T14:19:29Z

just cpp-test works on Mac

emilk · 2023-10-31T15:49:58Z

rerun_cpp/CMakeLists.txt

        --preset ninja-debug-minimal
-        -DARROW_IPC=ON
+        -DARROW_BOOST_USE_SHARED=OFF
        -DARROW_BUILD_SHARED=${ARROW_BUILD_SHARED}
        -DARROW_BUILD_STATIC=${ARROW_BUILD_STATIC}
-        -DCMAKE_INSTALL_PREFIX=${ARROW_DOWNLOAD_PATH}
+        -DARROW_CXXFLAGS=${DARROW_CXXFLAGS}
+        -DARROW_IPC=ON
+        -DARROW_JEMALLOC=OFF
+        -DARROW_MIMALLOC=ON
        -DARROW_USE_ASAN=OFF
        -DARROW_USE_TSAN=OFF
        -DARROW_USE_UBSAN=OFF
-        -DARROW_JEMALLOC=OFF
-        -Dxsimd_SOURCE=BUNDLED
        -DBOOST_SOURCE=BUNDLED
-        -DARROW_BOOST_USE_SHARED=OFF
-        -DARROW_CXXFLAGS=${DARROW_CXXFLAGS}
+        -DCMAKE_BUILD_TYPE=${ARROW_CMAKE_BUILD_TYPE}
+        -DCMAKE_INSTALL_PREFIX=${ARROW_DOWNLOAD_PATH}
+        -Dxsimd_SOURCE=BUNDLED


We should sort these defines if their order doesn't matter

This PR sorts them.

…BUILD_TYPE for debug/release

Wumpf · 2023-10-31T16:19:02Z

oh wow. CRLF strikes again. my settings are broken
edit: fixed

Wumpf

fixed for windows now, didn't run benchmarks though, only made sure build all & tests work

emilk · 2023-10-31T16:30:32Z

tip: there are global git settings you can set regarding CRLF

Wumpf · 2023-10-31T16:32:46Z

tip: there are global git settings you can set regarding CRLF

I know, that's the strange thing, I set autocrlf = false on my global .gitconfig and I still got this. Must be something else in VSCode that caused this because changing the endings and commting that worked fine 😕

rerun_cpp/CMakeLists.txt

emilk · 2023-10-31T16:35:22Z

rerun_cpp/src/rerun/demo_utils.hpp

@@ -30,7 +30,7 @@ namespace rerun {
        inline std::vector<T> linspace(T start, T end, size_t num) {
            std::vector<T> linspaced(num);
            std::generate(linspaced.begin(), linspaced.end(), [&, i = 0]() mutable {
-                return start + static_cast<T>(i++) * (end - start) / static_cast<T>(num - 1);
+                return static_cast<T>(start + static_cast<T>(i++) * (end - start) / static_cast<T>(num - 1));


this makes no sense to me - isn't this casting T to T? Are you sure this is needed?

Yeah it's this weird quirk of C++ where char + char == int

(you know that weird quirk. Not like the others)
Bites me every time!

https://godbolt.org/z/4W8xYnqKq

actually only works with unsigned char, which is also the case I got a warning in (uint8_t)

God damn you C++

jleibs · 2023-10-31T18:16:27Z

~~@emilk can you re-test on mac with the latest changes, and check the checkbox when done?~~ @Wumpf tested it

jleibs added 2 commits October 30, 2023 23:16

Pass through CMAKE_BUILD_TYPE to arrow build

8e51145

Sort

ff1d386

jleibs added 🌊 C++ API C or C++ logging API include in changelog labels Oct 30, 2023

jleibs added this to the 0.10.1 milestone Oct 30, 2023

jleibs marked this pull request as ready for review October 30, 2023 22:19

jleibs changed the title ~~Pass through CMAKE_BUILD_TYPE to arrow build~~ Pass through CMAKE_BUILD_TYPE to arrow build and enable mimalloc Oct 30, 2023

Enable mimalloc

4166ef9

jleibs changed the title ~~Pass through CMAKE_BUILD_TYPE to arrow build and enable mimalloc~~ Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc Oct 30, 2023

Default arrow to release if no build-type is otherwise specified

2af8f9d

teh-cmc approved these changes Oct 31, 2023

View reviewed changes

emilk reviewed Oct 31, 2023

View reviewed changes

rerun_cpp/CMakeLists.txt Show resolved Hide resolved

Merge branch 'main' into jleibs/arrow_match_build_flags

c68cfbf

emilk approved these changes Oct 31, 2023

View reviewed changes

Wumpf added 2 commits October 31, 2023 17:14

Fix windows issues and use arrow cmake prefix files instead of CMAKE_…

d80eeb8

…BUILD_TYPE for debug/release

fix windows warning

28ff7b0

fix CRLF

1504b38

Wumpf approved these changes Oct 31, 2023

View reviewed changes

emilk approved these changes Oct 31, 2023

View reviewed changes

Wumpf and others added 3 commits October 31, 2023 17:36

fix definitely not helpful link on cmakelists comment

c441936

Merge branch 'main' into jleibs/arrow_match_build_flags

d0d05b9

Use static bundled dependencies on all platforms

87c0346

Fix clean build

73f9733

jleibs merged commit a143e9d into main Oct 31, 2023
34 of 35 checks passed

jleibs deleted the jleibs/arrow_match_build_flags branch October 31, 2023 19:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc #4094

Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc #4094

jleibs commented Oct 30, 2023 •

edited by Wumpf

Wumpf commented Oct 31, 2023

emilk commented Oct 31, 2023

emilk Oct 31, 2023

jleibs Oct 31, 2023

Wumpf commented Oct 31, 2023 •

edited

Wumpf left a comment

emilk commented Oct 31, 2023

Wumpf commented Oct 31, 2023

emilk Oct 31, 2023

Wumpf Oct 31, 2023

Wumpf Oct 31, 2023

emilk Oct 31, 2023

jleibs commented Oct 31, 2023 •

edited

Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc #4094

Improve C++ SDK perf 5x by respecting CMAKE_BUILD_TYPE and enabling mimalloc #4094

Conversation

jleibs commented Oct 30, 2023 • edited by Wumpf

What

Test

Checklist

Wumpf commented Oct 31, 2023

emilk commented Oct 31, 2023

emilk Oct 31, 2023

Choose a reason for hiding this comment

jleibs Oct 31, 2023

Choose a reason for hiding this comment

Wumpf commented Oct 31, 2023 • edited

Wumpf left a comment

Choose a reason for hiding this comment

emilk commented Oct 31, 2023

Wumpf commented Oct 31, 2023

emilk Oct 31, 2023

Choose a reason for hiding this comment

Wumpf Oct 31, 2023

Choose a reason for hiding this comment

Wumpf Oct 31, 2023

Choose a reason for hiding this comment

emilk Oct 31, 2023

Choose a reason for hiding this comment

jleibs commented Oct 31, 2023 • edited

jleibs commented Oct 30, 2023 •

edited by Wumpf

Wumpf commented Oct 31, 2023 •

edited

jleibs commented Oct 31, 2023 •

edited