Switch back to O0. #1211

vgvassilev · 2017-10-20T08:30:53Z

We have a suboptimal behavior in the way cling optimizes code in O2 mode.
Disable it until the issue is understood and fixed.

vgvassilev · 2017-10-20T08:31:20Z

@phsft-bot build with flags -Dclingtest=On

phsft-bot · 2017-10-20T08:31:21Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-10-20T08:31:59Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

phsft-bot · 2017-10-20T09:18:13Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T09:22:45Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

phsft-bot · 2017-10-20T09:24:46Z

Build failed on mac1012/native.
See console output.

Warnings:

/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]

And 10 more

Failing tests:

projectroot.interpreter.clingtest_check_cling

We have a suboptimal behavior in the way cling optimizes code in O2 mode. Disable it until the issue is understood and fixed.

eguiraud · 2017-10-20T09:27:16Z

Hi,
here are flamegraphs of the following simple benchmark with current ROOT master (test_O2.svg) and then with this PR (test_O0.svg). Runtime goes from 9s to 3s and the part of the stack containing cling::IncrementalParser::Compile up to cling::Intepreter::ExecuteTransaction disappears/seems to weight much less.

#include "ROOT/TDataFrame.hxx"
#include <iostream>
using namespace ROOT::Experimental;

int main() {
   // build a TDF with 1 event and 1 column "x" that is always equal 42
   TDataFrame dd(1);
   auto d = dd.Define("x", []() { return 42; });

   // book nHistos histograms
   // all with the same cut and filled with the same variable in this simple example
   std::vector<TDF::TResultProxy<TH1D>> histos;
   const auto nHistos = 1000u;
   histos.reserve(nHistos);
   for (auto i = 0u; i < nHistos; ++i)
      histos.emplace_back(d.Histo1D("x"));

   // run event loop, print something to be sure everything is ok
   // jitting of the 1000 booked histograms happens here
   std::cout << histos.front()->GetMean() << std::endl;
   return 0;
}

Test -O0

Test -O2

phsft-bot · 2017-10-20T09:29:20Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

vgvassilev · 2017-10-20T09:30:29Z

@pcanal, git blame shows that you are the one with most recent knowledge about cling/test/Autoloading/AutoForwarding.C.

Independent on this PR, when we build with -Dclingtest=On that test fails. I think it could be a valid regression happened because we do not really test that part of the code in cling standalone. Could you take a look?

dpiparo · 2017-10-20T09:37:46Z

@vgvassilev nice job. The benchmark I have creating hundreds of histos relying on jitting takes w/o this patch 12s and 3s w/ it. For me the code good to be merged.

vgvassilev · 2017-10-20T09:42:43Z

@bluehood thanks for the graphs. Looks like llvm::FPPassManager::runOnModule is much hotter than its O0 counterpart. Can we get some sort of number of calls for these functions?

eguiraud · 2017-10-20T09:54:36Z

@vgvassilev I think the most notable difference is the absence of cling::Intepreter::executeTransaction (and deeper parts of that stack) in the -O0 run.

In any case, I will run callgrind on both versions.

phsft-bot · 2017-10-20T10:31:22Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:33:36Z

Build failed on centos7/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T10:57:04Z

Build failed on ubuntu14/native.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T11:05:38Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T11:10:07Z

Build failed on slc6/gcc62.
See console output.

Warnings:

googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1522:28: warning: the compiler can assume that the address of ‘val1’ will never be NULL [-Waddress]
googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1510:3: warning: nonnull argument ‘val1’ compared to NULL [-Wnonnull-compare]

Failing tests:

projectroot.interpreter.clingtest_check_cling

phsft-bot · 2017-10-20T11:11:04Z

Build failed on mac1012/native.
See console output.

Warnings:

/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
/cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1012-clang81-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]

And 10 more

Failing tests:

projectroot.interpreter.clingtest_check_cling

eguiraud · 2017-10-20T11:21:31Z

Please find attached callgrind outputs for the O0 and O2 runs of the snippet listed in my previous comment: callgrind.zip.

phsft-bot · 2017-10-20T11:24:27Z

Build failed on slc6/gcc62.
See console output.

Warnings:

googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1522:28: warning: the compiler can assume that the address of ‘val1’ will never be NULL [-Waddress]
googletest-prefix/src/googletest/googletest/include/gtest/gtest.h:1510:3: warning: nonnull argument ‘val1’ compared to NULL [-Wnonnull-compare]

Failing tests:

projectroot.interpreter.clingtest_check_cling

dpiparo · 2017-11-02T07:59:26Z

Hi, I vote to merge this asap.

vgvassilev · 2017-11-02T08:54:51Z

I'd like to put that in after PR #1218. I expect that would happen by the end of the week.

vgvassilev · 2017-11-03T08:38:53Z

@phsft-bot build!

phsft-bot · 2017-11-03T08:39:26Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu14/native, ubuntu14/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

vgvassilev requested a review from Axel-Naumann as a code owner October 20, 2017 08:30

vgvassilev requested review from dpiparo and eguiraud October 20, 2017 08:31

Switch back to O0.

4b3dd1a

We have a suboptimal behavior in the way cling optimizes code in O2 mode. Disable it until the issue is understood and fixed.

vgvassilev force-pushed the RevertToO0 branch from 7a032be to 4b3dd1a Compare October 20, 2017 09:28

dpiparo approved these changes Oct 20, 2017

View reviewed changes

vgvassilev merged commit 83060bd into root-project:master Nov 3, 2017

vgvassilev deleted the RevertToO0 branch November 3, 2017 13:13

phsft-bot mentioned this pull request May 17, 2023

[TCling] C++-use string,vector instead of std. #11027

Draft

Switch back to O0. #1211

Switch back to O0. #1211

Uh oh!

Conversation

vgvassilev commented Oct 20, 2017

Uh oh!

vgvassilev commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

eguiraud commented Oct 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phsft-bot commented Oct 20, 2017

Uh oh!

vgvassilev commented Oct 20, 2017

Uh oh!

dpiparo commented Oct 20, 2017

Uh oh!

vgvassilev commented Oct 20, 2017

Uh oh!

eguiraud commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

eguiraud commented Oct 20, 2017

Uh oh!

phsft-bot commented Oct 20, 2017

Warnings:

Failing tests:

Uh oh!

dpiparo commented Nov 2, 2017

Uh oh!

vgvassilev commented Nov 2, 2017

Uh oh!

vgvassilev commented Nov 3, 2017

Uh oh!

phsft-bot commented Nov 3, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eguiraud commented Oct 20, 2017 •

edited

Loading