Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[DF] Remove false sharing in RFilter, RDefine #8007

Merged
merged 2 commits into from
Apr 29, 2021
Merged

[DF] Remove false sharing in RFilter, RDefine #8007

merged 2 commits into from
Apr 29, 2021

Conversation

eguiraud
Copy link
Member

Use larger vectors and access them more sparsely if needed (e.g.
threads now access one every 16 elements in hot std::vector
on systems where a cache line is 64 bytes).

In some edge cases in which the analysis workload is light and/or many
threads (48, 64) are involved, this patch reduces runtimes by factors.
The increase in memory usage should be little relative to the memory
required e.g. by jitting or by per-thread histograms.

@eguiraud eguiraud self-assigned this Apr 27, 2021
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-27T11:28:55.053Z] FAILED: tree/dataframe/G__ROOTDataFrame.cxx lib/ROOTDataFrame.pcm
  • [2021-04-27T11:28:55.999Z] /Users/sftnight/build/workspace/root-pullrequests-build/build/include/ROOT/RDF/Utils.hxx:211:55: error: no member named 'hardware_destructive_interference_size' in namespace 'std'
  • [2021-04-27T11:28:55.999Z] /Users/sftnight/build/workspace/root-pullrequests-build/build/include/ROOT/RDF/Utils.hxx:215:53: error: constexpr variable 'kCacheLineLong64Step' must be initialized by a constant expression
  • [2021-04-27T11:28:55.999Z] /Users/sftnight/build/workspace/root-pullrequests-build/build/include/ROOT/RDF/Utils.hxx:216:50: error: constexpr variable 'kCacheLineIntStep' must be initialized by a constant expression

Axel-Naumann
Axel-Naumann approved these changes Apr 27, 2021
View reviewed changes
Copy link
Member

@Axel-Naumann Axel-Naumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion to reduce the amount of names :-)

tree/dataframe/inc/ROOT/RDF/Utils.hxx Outdated Show resolved Hide resolved
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-27T15:06:45.193Z] FAILED: tree/dataframe/G__ROOTDataFrame.cxx lib/ROOTDataFrame.pcm
  • [2021-04-27T15:06:46.620Z] /Users/sftnight/build/workspace/root-pullrequests-build/build/include/ROOT/RDF/Utils.hxx:211:55: error: no member named 'hardware_destructive_interference_size' in namespace 'std'

@eguiraud
Copy link
Member Author

Nice, mac11@C++17 is missing C++17 features

@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@eguiraud @Axel-Naumann
[DF] Remove false sharing in RFilter, RDefine
d59e6e0 
Use larger vectors and access them more sparsely if needed (e.g.
threads now access one every 16 elements in hot std::vector<int>
on systems where a cache line is 64 bytes).

In some edge cases in which the analysis workload is light and/or many
threads (48, 64) are involved, this patch reduces runtimes by factors.
The increase in memory usage should be little relative to the memory
required e.g. by jitting or by per-thread histograms.

Co-authored-by: Axel Naumann <Axel.Naumann@cern.ch>
@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-28T08:39:17.421Z] FAILED: tree/dataframe/test/MaxSlotHelperDict.cxx tree/dataframe/test/libMaxSlotHelperDict_rdict.pcm tree/dataframe/test/libMaxSlotHelperDict.rootmap
  • [2021-04-28T08:39:17.421Z] /Users/sftnight/build/workspace/root-pullrequests-build/build/include/ROOT/RDF/Utils.hxx:209:55: error: no member named 'hardware_destructive_interference_size' in namespace 'std'

@eguiraud eguiraud requested a review from oshadura as a code owner April 28, 2021 11:01
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

Axel-Naumann
Axel-Naumann approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@Axel-Naumann Axel-Naumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might suggest to change the check, either way: looks great!

cmake/modules/RootConfiguration.cmake Outdated Show resolved Hide resolved
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

@eguiraud
Copy link
Member Author

@phsft-bot build just on mac11.0/cxx17

@phsft-bot
Copy link
Collaborator

Starting build on mac11.0/cxx17
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2021-04-28T21:30:10.701Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:194:21: warning: 'destroy' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.701Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:128:24: warning: 'destroy' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.701Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:99:21: warning: 'construct' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:99:21: warning: 'construct<double>' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:140:54: warning: 'max_size' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:99:21: warning: 'construct' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:99:21: warning: 'construct<double, double>' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:128:24: warning: 'destroy' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:140:54: warning: 'max_size' is deprecated [-Wdeprecated-declarations]
  • [2021-04-28T21:30:10.702Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/math/vecops/inc/ROOT/RAdoptAllocator.hxx:99:21: warning: 'construct' is deprecated [-Wdeprecated-declarations]

And 1475 more

Failing tests:

@eguiraud
[DF] Use ad-hoc macro instead of broken library feature
973f30c 
Implement and use `R__HAS_HARDWARE_INTERFERENCE_SIZE` instead of the
standard `__cpp_lib_hardware_interference_size` to check for support
of `std::hardware_destructive_interference_size`: at least on Mac 11,
libc++ defines that macro but is missing the actual feature (see
llvm/llvm-project@174322c).
@eguiraud eguiraud merged commit f0e77a6 into root-project:master Apr 29, 2021
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@eguiraud eguiraud deleted the df_false_sharing branch April 29, 2021 19:38
@phsft-bot
Copy link
Collaborator

Build failed on mac1014/python3.
Running on macphsft17.dyndns.cern.ch:/build/jenkins/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-29T19:42:03.397Z] CMake Error at /build/jenkins/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1038 (message):

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-29T19:43:24.570Z] CMake Error at /Users/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1038 (message):

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-29T20:55:52.496Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1038 (message):

@phsft-bot
Copy link
Collaborator

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

  • [2021-04-30T01:04:53.336Z] CMake Error at C:/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1038 (message):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Axel-Naumann Axel-Naumann Axel-Naumann approved these changes

@stwunsch stwunsch Awaiting requested review from stwunsch

@oshadura oshadura Awaiting requested review from oshadura

Assignees

@eguiraud eguiraud

Labels
pr:squash on merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eguiraud @phsft-bot @Axel-Naumann