New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CMake] add liburing build option #5919
Conversation
Starting build on |
Build failed on mac1014/python3. Failing tests: |
Build failed on windows10/cxx14. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! I have other suggestions how to improve a bit more build system, but I think it will be in next iteration of review :)
@@ -0,0 +1,23 @@ | |||
# Copyright (C) 1995-2020, Rene Brun and Fons Rademakers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found other nice example https://github.com/tkowalcz/io_uring_tutorial/blob/master/FindLibUring.cmake
Starting build on |
Starting build on |
Build failed on windows10/cxx14. |
Build failed on ROOT-performance-centos7-multicore/default. Failing tests: |
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
Starting build on |
Build failed on windows10/cxx14. |
Starting build on |
Build failed on windows10/cxx14. |
Build failed on mac1015/cxx17. Failing tests: |
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
Build failed on mac1014/python3. Failing tests: |
Build failed on ROOT-performance-centos7-multicore/default. Failing tests: |
Build failed on ROOT-fedora31/noimt. Failing tests: |
Starting build on |
Build failed on windows10/cxx14. |
Build failed on ROOT-fedora31/noimt. Failing tests: |
Build failed on ROOT-performance-centos7-multicore/default. Failing tests: |
Build failed on mac1015/cxx17. Failing tests: |
Build failed on mac1014/python3. Failing tests:
|
Build failed on ROOT-fedora30/cxx14. Failing tests: |
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests:
And 37 more |
Starting build on |
All errors seem unrelated. @etejedor will hopefully look at the Python errors when he's back. And I haven't seen the cling symbol errors in my PR - which is why I merged it, and apparently I broke cling :-( I'll have to debug tomorrow... |
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests:
And 37 more |
Build failed on mac1014/python3. Failing tests: |
io/io/test/CMakeLists.txt
Outdated
@@ -9,3 +9,6 @@ ROOT_ADD_GTEST(TFile TFileTests.cxx LIBRARIES RIO) | |||
ROOT_ADD_GTEST(TBufferMerger TBufferMerger.cxx LIBRARIES RIO Imt Tree) | |||
ROOT_ADD_GTEST(TFileMerger TFileMergerTests.cxx LIBRARIES RIO Tree) | |||
ROOT_ADD_GTEST(TROMemFile TROMemFileTests.cxx LIBRARIES RIO Tree) | |||
if(uring AND NOT DEFINED ENV{ROOTTEST_IGNORE_URING}) | |||
ROOT_ADD_GTEST(RIoUring RIoUring.cxx LIBRARIES RIO uring) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't uring
inherited through RIO
? I'd hope that you don't need to spell out LIBRARIES uring
here:
ROOT_ADD_GTEST(RIoUring RIoUring.cxx LIBRARIES RIO uring) | |
ROOT_ADD_GTEST(RIoUring RIoUring.cxx LIBRARIES RIO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The missing piece here is that I didn't make target_link_libraries
PUBLIC
instead of PRIVATE
. I think it is fixed in 23d2a9f
Starting build on |
@phsft-bot build also on ROOT-fedora32/default with flags -During=ON |
Starting build on |
Build failed on ROOT-fedora30/cxx14. Errors:
|
Build failed on ROOT-fedora31/noimt. Errors:
|
I thought this would only build |
NP! FYI that's |
Build failed on ROOT-fedora32/default. Warnings:
Failing tests:
|
Build failed on ROOT-debian10-i386/cxx14. Errors:
|
Build failed on ROOT-performance-centos7-multicore/default. Errors:
|
Build failed on ROOT-ubuntu16/nortcxxmod. Errors:
|
@Axel-Naumann the tests on |
Thanks, Max. Do you want me to merge or do you prefer to wait for Jakob? |
I am definitely OK waiting for Jakob. I believe all the outstanding issues are fixed up, but waiting one more day doesn't block anything (I can keep going on my local branch and make a new PR for that). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Can't wait to benchmark SSD performance with and without async I/O.
@Axel-Naumann I think all is addressed, please approve if it looks good to you |
This PR allows us to use
liburing
, the "application-level" interface toio_uring
, the new Linux kernel IO interface.liburing
takes care of many of the low-level details (e.g.mmap
calls, barriers) required to correctly useio_uring
.io_uring
is supported on Linux kernels 5.1 and up, but it's up to the user to download and installliburing
themselves, either from source (make && make install
) or through a package manager.The rationale for including
liburing
in ROOT is to experiment withio_uring
's parallel, async IO features. This may drive performance improvements in low-level IO.I based the CMake changes on how
jemalloc
is handled. I am not very experienced with CMake and would be happy to find out that I've done something wrong here. We need to able to include (at least) twoliburing
header files and link against the shared library, namely#include "liburing.h"
#include "liburing/io_uring.h"
liburing.so