Skip to content

Set cling optimization level to 3 and turn off pointer checks #6038

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
Closed
LukasBreitwieser wants to merge 1 commit into from
Closed

Set cling optimization level to 3 and turn off pointer checks #6038

LukasBreitwieser wants to merge 1 commit into from

Conversation

@LukasBreitwieser
Copy link
Contributor

@LukasBreitwieser LukasBreitwieser commented Jul 15, 2020

Our use case requires optimization level 3 and disabled pointer checks. Therefore Fons asked me to send this PR.
Please see the following discussion:
https://root-forum.cern.ch/t/jit-performance-issue/38555

Commit based on
https://bitbucket.org/wlav/cppyy-backend/src/master/cling/patches/optlevel2_forced.diff

@phsft-bot
Copy link

phsft-bot commented Jul 15, 2020

Can one of the admins verify this patch?

LukasBreitwieser
LukasBreitwieser commented Jul 15, 2020
View reviewed changes
interpreter/cling/lib/Interpreter/IncrementalParser.cpp
ASTTransformers.emplace_back(new AutoSynthesizer(TheSema));
ASTTransformers.emplace_back(new EvaluateTSynthesizer(TheSema));
if (hasCodeGenerator() && !m_Interpreter->getOptions().NoRuntime) {
if (0 && hasCodeGenerator() && !m_Interpreter->getOptions().NoRuntime) {
Copy link
Contributor Author

@LukasBreitwieser LukasBreitwieser Jul 15, 2020

Choose a reason for hiding this comment

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

Which approach do you recommend to turn off pointer checks?

@pcanal
Copy link
Member

pcanal commented Jul 15, 2020

This PR increases the optimization level (in many cases, this lead to more time spend in loading the script that executing it) and disable nullptr pointer check for *everybody* is that really the intent?

@LukasBreitwieser
Copy link
Contributor Author

LukasBreitwieser commented Jul 21, 2020

Hi Philippe,
The PR shows the settings that we need for the BioDynaMo project (agent-based simulation framework).
We JIT a few functors at the beginning of the simulation and call them a couple hundred million times during simulation.

The intent of this PR is to start a discussion. Fons dislikes the idea of using a patched ROOT version.

Until https://sft.its.cern.ch/jira/browse/ROOT-10707 ships, would it be possible to add an e.g. rootrc parameter to influence the optimization level and pointer checks, or do you recommend waiting for ROOT-10707?

@oshadura
Copy link
Collaborator

oshadura commented Aug 5, 2020

@phsft-bot build!

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

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

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

And 714 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

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

Failing tests:

And 726 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

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

Failing tests:

And 733 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

Build failed on ROOT-fedora30/cxx14.
Running on root-fedora30-1.cern.ch:/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

And 723 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

Build failed on ROOT-performance-centos7-multicore/default.
Running on olhswep22.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See cdash.
See console output.

Errors:

  • [2020-08-05T09:03:45.341Z] 25/2067 Test Make ReadClass error message more verbose #911: tutorial-unfold-testUnfold5a ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.23 sec
  • [2020-08-05T09:03:45.342Z] 31/2067 Test string_view-related changes #854: tutorial-tmva-TMVAClassification ..................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.32 sec
  • [2020-08-05T09:03:46.945Z] 43/2067 Test First Patch RNN #685: tutorial-multicore-mp104_processH1 ................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.58 sec
  • [2020-08-05T09:03:47.516Z] 45/2067 Test [TDF] Misc fixes #867: tutorial-tmva-TMVARegression ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.40 sec
  • [2020-08-05T09:03:48.811Z] 73/2067 Test CMake integration with external projects #916: tutorial-unfold-testUnfold7a ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 4.59 sec
  • [2020-08-05T09:03:49.070Z] 78/2067 Test [genreflex] Add support for noStreamer and noInputOperator xml attribute #864: tutorial-tmva-TMVAMulticlass ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.40 sec
  • [2020-08-05T09:04:26.591Z] 639/2067 Test Fix for test suite on Fedora 26 i686 #359: tutorial-dataframe-df003_profiles .................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.58 sec
  • [2020-08-05T09:04:28.151Z] 654/2067 Test Add a class to perform compensate summation #372: tutorial-dataframe-df017_vecOpsHEP ................................................................***Failed Error regular expression found in output. Regex=[: error:] 3.43 sec
  • [2020-08-05T09:04:28.926Z] 671/2067 Test [TDF] miscellaneous improvements #387: tutorial-dataframe-df102_NanoAODDimuonAnalysis ....................................................***Failed Error regular expression found in output. Regex=[: error:] 2.51 sec
  • [2020-08-05T09:04:36.624Z] 750/2067 Test Fix inf loop in test on arm #450: tutorial-geom-runplugin ...........................................................................***Failed Error regular expression found in output. Regex=[: error:] 5.67 sec

And 8 more

Failing tests:

And 1237 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

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

Failing tests:

And 635 more

@phsft-bot
Copy link

phsft-bot commented Aug 5, 2020

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-4.cern.ch:/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

And 728 more

@hageboeck
Copy link
Member

hageboeck commented Sep 14, 2020

Converted to draft in order to be able to remove the do not merge label.

@hageboeck hageboeck marked this pull request as draft September 14, 2020 14:52
@Axel-Naumann
Copy link
Member

Axel-Naumann commented Apr 27, 2022

This should be addressed in 6.26 and master. Can you confirm the effect?

Given that we now have a mechanism in place I will close this PR.

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

Reviewers

@Axel-Naumann Axel-Naumann Awaiting requested review from Axel-Naumann

@pcanal pcanal Awaiting requested review from pcanal pcanal is a code owner

@dpiparo dpiparo Awaiting requested review from dpiparo dpiparo will be requested when the pull request is marked ready for review dpiparo is a code owner

Assignees

@Axel-Naumann Axel-Naumann

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@LukasBreitwieser @phsft-bot @pcanal @oshadura @hageboeck @Axel-Naumann