[cxxmodules] Fix failing runtime_cxxmodules tests by preloading modules #1814

yamaguchi1024 · 2018-04-03T12:49:38Z

Currently, 36 tests are failing for runtime modules:
https://epsft-jenkins.cern.ch/view/ROOT/job/root-nightly-runtime-cxxmodules/
We want to make these test pass so that we can say that the runtime modules is
finally working.

This patch enables ROOT to preload all modules at startup time. In my
environment, this patch fixes 14 tests for runtime cxxmodules.

Preloading all the modules has several advantages. 1. We do not have to
rely on rootmap files which don't support some features (namespaces and
templates) 2. Lookup would be faster because we don't have to do
trampoline via rootmap files.

The only disadvantage of preloading all the modules is the startup time performance.
root.exe -q -l memory.C
This is a release build without modules:

 cpu  time = 0.091694 seconds
 sys  time = 0.026187 seconds
 res  memory = 133.008 Mbytes
 vir  memory = 217.742 Mbytes

This is a release build with modules, with this patch:

 cpu  time = 0.234134 seconds
 sys  time = 0.066774 seconds
 res  memory = 275.301 Mbytes
 vir  memory = 491.832 Mbytes

As you can see, preloading all the modules makes both time and memory 2
to 3 times worse at a startup time.

Edit : With hsimple.C
root.exe -l -b tutorials/hsimple.C -q ~/CERN/ROOT/memory.C
Release build without modules:

Processing tutorials/hsimple.C...                                                                        
hsimple   : Real Time =   0.04 seconds Cpu Time =   0.05 seconds                        
(TFile *) 0x555ae2a9d560                                                                  
Processing /home/yuka/CERN/ROOT/memory.C...                                                              
 cpu  time = 0.173591 seconds                                   
 sys  time = 0.011835 seconds                       
 res  memory = 135.32 Mbytes                                    
 vir  memory = 209.664 Mbytes

Release build with modules, with this patch:

Processing tutorials/hsimple.C...
hsimple   : Real Time =   0.04 seconds Cpu Time =   0.04 seconds
(TFile *) 0x55d1b036d230
Processing /home/yuka/CERN/ROOT/memory.C...
 cpu  time = 0.290742 seconds
 sys  time = 0.043851 seconds
 res  memory = 256.844 Mbytes
 vir  memory = 438.484 Mbytes

However, it is a matter of course that we get slower startup time if we
try to load all the modules at startup time, not on-demand. I haven't had a good benchmark for this but, in theory, it reduces execution time instead as we're anyway loading modules after the startup.

yamaguchi1024 · 2018-04-03T13:10:04Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

vgvassilev · 2018-04-03T13:13:14Z

core/metacling/src/TCling.cxx

@@ -1258,7 +1274,32 @@ TCling::TCling(const char *name, const char *title)
      // Load libc and stl first.
      LoadModules({"libc", "stl"}, *fInterpreter);

-      LoadModules({"ROOT_Foundation_C", "ROOT_Config", "ROOT_Foundation_Stage1_NoRTTI", "Core", "RIO"}, *fInterpreter);
+      // Load core modules
+      std::vector<std::string> CoreModules = {"ROOT_Foundation_C","ROOT_Config","ROOT_Foundation_Stage1_NoRTTI", "Core", "RIO"};


Maybe that'd be better being a std::set<const char*>?

vgvassilev · 2018-04-03T13:14:22Z

core/metacling/src/TCling.cxx

+            if (!ModuleName.empty() && std::find(CoreModules.begin(), CoreModules.end(), ModuleName) == CoreModules.end()) {
+               if (M->IsSystem && !M->IsMissingRequirement)
+                  LoadModule(ModuleName, *fInterpreter);
+               else if (!M->IsSystem && !M->IsMissingRequirement)


We should check for filenames, i.e. if the module is present on disk as a pcm file.

vgvassilev · 2018-04-03T13:24:19Z

core/metacling/src/TCling.cxx

+      LoadModules(CoreModules, *fInterpreter);
+
+      // Otherwise module conflicts between rootcling and root
+      if (!fromRootCling) {


This check can likely go away if we check if the module is already built (i.e. if it has an on-disk representing pcm). This way rootcling will become more efficient by building a module, while reusing the previously build ones.

vgvassilev · 2018-04-03T13:38:58Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

vgvassilev · 2018-04-03T14:36:12Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

phsft-bot · 2018-04-03T14:37:22Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

phsft-bot · 2018-04-03T17:50:20Z

Build failed on slc6/gcc62.
See console output.

Errors:

299/1675 Test Master tmva gsoc2016 #180: tutorial-benchmarks ...............................................................................***Failed Error regular expression found in output. Regex=[: error:] 66.76 sec
433/1675 Test [RooFit] Add missing 'add' functions to RooArgSet. #325: tutorial-graphics-tornado .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.94 sec
470/1675 Test Add a chunking interface to TProcessExecutor #364: tutorial-hist-ContourList .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 3.27 sec
684/1675 Test C++17 fixes #591: tutorial-roostats-HybridInstructional .............................................................***Failed Error regular expression found in output. Regex=[: error:] 4.91 sec
686/1675 Test Add support for LZ4 in classic build #593: tutorial-roostats-HybridStandardForm ..............................................................***Failed Error regular expression found in output. Regex=[: error:] 4.06 sec

Failing tests:

And 86 more

phsft-bot · 2018-04-03T20:05:37Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

yamaguchi1024 · 2018-04-03T20:06:38Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

phsft-bot · 2018-04-03T20:06:44Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

phsft-bot · 2018-04-03T21:07:58Z

Build failed on slc6/gcc62.
See console output.

Errors:

298/1675 Test Documentation fixes fixes #184: tutorial-dataframe-tdf002_dataModel ...............................................................***Failed Error regular expression found in output. Regex=[: error:] 9.81 sec
304/1675 Test Force ON LLVM_ENABLE_CXX1Y when cxx14 is enabled. #195: tutorial-dataframe-tdf013_InspectAnalysis .........................................................***Failed Error regular expression found in output. Regex=[: error:] 0.52 sec
308/1675 Test First GPU implementation of DNNs. #199: tutorial-dataframe-tdf017_vecOpsHEP ...............................................................***Failed Error regular expression found in output. Regex=[: error:] 1.24 sec
560/1675 Test Fix ABI compability when linking against OCCI when using clang. #460: tutorial-math-quasirandom .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.12 sec
586/1675 Test [WIP] Allow hadd to run in multiprocess mode #491: tutorial-multicore-mtbb201_parallelHistoFill ......................................................***Failed Error regular expression found in output. Regex=[: error:] 2.99 sec
702/1675 Test TMVA various fixes #610: tutorial-roostats-Zbi_Zgamma ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.15 sec
714/1675 Test Extend TCanvasImp interface #622: tutorial-roostats-rs_numbercountingutils ..........................................................***Failed Error regular expression found in output. Regex=[: error:] 1.23 sec
796/1675 Test Updated gitignore for QtCreator files. #715: tutorial-dataframe-tdf002_dataModel-py ............................................................***Failed Error regular expression found in output. Regex=[: error:] 0.92 sec

Failing tests:

And 32 more

phsft-bot · 2018-04-03T22:19:28Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

yamaguchi1024 · 2018-04-03T22:19:43Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=On

phsft-bot · 2018-04-03T22:19:49Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=On
How to customize builds

phsft-bot · 2018-04-03T23:25:14Z

Build failed on slc6/gcc62.
See console output.

Errors:

299/1675 Test Master tmva gsoc2016 #180: tutorial-benchmarks ...............................................................................***Failed Error regular expression found in output. Regex=[: error:] 21.54 sec
433/1675 Test [RooFit] Add missing 'add' functions to RooArgSet. #325: tutorial-graphics-tornado .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.20 sec
473/1675 Test Add a chunking interface to TProcessExecutor #364: tutorial-hist-ContourList .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.50 sec
684/1675 Test C++17 fixes #591: tutorial-roostats-HybridInstructional .............................................................***Failed Error regular expression found in output. Regex=[: error:] 1.55 sec
687/1675 Test Add support for LZ4 in classic build #593: tutorial-roostats-HybridStandardForm ..............................................................***Failed Error regular expression found in output. Regex=[: error:] 1.76 sec

Failing tests:

And 84 more

yamaguchi1024 · 2018-04-04T08:23:12Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=On -DCMAKE_BUILD_TYPE=Release

phsft-bot · 2018-04-04T08:30:45Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=On -DCMAKE_BUILD_TYPE=Release
How to customize builds

phsft-bot · 2018-04-04T09:25:32Z

Build failed on slc6/gcc62.
See console output.

Errors:

301/1677 Test Master tmva gsoc2016 #180: tutorial-benchmarks ...............................................................................***Failed Error regular expression found in output. Regex=[: error:] 14.37 sec
434/1677 Test [RooFit] Add missing 'add' functions to RooArgSet. #325: tutorial-graphics-tornado .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.43 sec
470/1677 Test Add a chunking interface to TProcessExecutor #364: tutorial-hist-ContourList .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.01 sec
684/1677 Test C++17 fixes #591: tutorial-roostats-HybridInstructional .............................................................***Failed Error regular expression found in output. Regex=[: error:] 1.34 sec
688/1677 Test Add support for LZ4 in classic build #593: tutorial-roostats-HybridStandardForm ..............................................................***Failed Error regular expression found in output. Regex=[: error:] 1.19 sec

Failing tests:

And 84 more

phsft-bot · 2018-04-04T09:32:14Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=On -DCMAKE_BUILD_TYPE=Release
How to customize builds

yamaguchi1024 · 2018-04-04T09:37:27Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=On

phsft-bot · 2018-04-04T09:37:33Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=On
How to customize builds

yamaguchi1024 · 2018-04-04T10:42:29Z

@phsft-bot build just on slc6/gcc62 with flags -DLLVM_BUILD_TYPE=Debug -Droofit=Off -Druntime_cxxmodules=On -DCMAKE_VERBOSE_MAKEFILE:BOOL=On -Dctest_test_exclude_none=On

phsft-bot · 2018-04-04T10:43:01Z

Starting build on slc6/gcc62 with flags -Dvc=OFF -Dimt=ON -Dccache=ON -DLLVM_BUILD_TYPE=Debug -Droofit=Off -Druntime_cxxmodules=On BOOL=On -Dctest_test_exclude_none=On
How to customize builds

yamaguchi1024 · 2018-04-04T10:55:22Z

@phsft-bot build just on slc6/gcc62 with flags -DLLVM_BUILD_TYPE=Debug -Droofit=Off -Druntime_cxxmodules=On -DCMAKE_VERBOSE_MAKEFILE:BOOL=On -Dctest_test_exclude_none=On

phsft-bot · 2018-05-17T15:47:13Z

Starting build on slc6/clang39, slc6/gcc48, slc6/gcc48, slc6/gcc49, slc6/gcc52, centos7/gcc62, centos7/gcc7, centos7/gcc7, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2018-05-17T16:08:45Z

Build failed on ubuntu16/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:09:03Z

Build failed on centos7/gcc7.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:10:29Z

Build failed on centos7/gcc62.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:18:32Z

Build failed on slc6/gcc48.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:22:18Z

Build failed on centos7/gcc7.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:28:44Z

Build failed on slc6/gcc52.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:29:34Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:31:39Z

Build failed on slc6/gcc48.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T16:59:09Z

Build failed on slc6/clang39.
See console output.

Warnings:

../root/core/metacling/src/TCling.cxx:1307:11: warning: suggest braces around initialization of subobject [-Wmissing-braces]

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T17:16:19Z

Build failed on mac1013/native.
See console output.

Warnings:

/Volumes/HD2/build/jenkins/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:1307:11: warning: suggest braces around initialization of subobject [-Wmissing-braces]

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

…ized decls Preloading all the modules has several advantages. 1. We do not have to rely on rootmap files which don't support some features (namespaces and templates) 2. Lookup would be faster because we don't have to do trampoline via rootmap files. Autoloading libraries when decls are deserialized gives us correctness. However we still need to optimize the performance by reducing the amount of loaded libraries and improving Clang performance.

phsft-bot · 2018-05-17T22:00:02Z

Starting build on slc6/clang39, slc6/gcc48, slc6/gcc48, slc6/gcc49, slc6/gcc52, centos7/gcc62, centos7/gcc7, centos7/gcc7, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2018-05-17T22:21:32Z

Build failed on ubuntu16/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:24:55Z

Build failed on centos7/gcc7.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:25:55Z

Build failed on centos7/gcc62.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:29:43Z

Build failed on slc6/gcc48.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:31:53Z

Build failed on slc6/clang39.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:33:27Z

Build failed on slc6/gcc52.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:33:44Z

Build failed on mac1013/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:37:50Z

Build failed on slc6/gcc48.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T22:38:05Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

phsft-bot · 2018-05-17T23:07:28Z

Build failed on centos7/gcc7.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto

vgvassilev

LGTM!

pcanal · 2018-05-21T16:03:26Z

Could a comment be added here on why the failure "projectroot.roottest.root.meta.roottest_root_meta_execUnloading_auto" does not prevent merging? (i.e. what is the plan to resolve it (upcoming fix in ROOT or ROOTTEST or revert this PR or something else?) ... in the meantime this will show up in all master builds including new PRs ... )

pcanal · 2018-05-21T18:13:56Z

To answer my own question:

The test needed to be updated and was updated by root-project/roottest#180.

Several PR and incremental failed due to the (unavoidable) lag between this PR being merged in and the roottest PR being merged in.

yamaguchi1024 self-assigned this Apr 3, 2018

yamaguchi1024 requested review from Axel-Naumann, vgvassilev and dpiparo April 3, 2018 12:49

yamaguchi1024 requested a review from pcanal as a code owner April 3, 2018 12:49

vgvassilev reviewed Apr 3, 2018

View reviewed changes

yamaguchi1024 force-pushed the allmodules branch from 6c6900e to 4c864d1 Compare April 4, 2018 09:31

yamaguchi1024 force-pushed the allmodules branch from 7070607 to d793877 Compare May 17, 2018 21:53

vgvassilev approved these changes May 18, 2018

View reviewed changes

vgvassilev merged commit 180c3a8 into root-project:master May 18, 2018

yamaguchi1024 deleted the allmodules branch May 18, 2018 08:25

yamaguchi1024 mentioned this pull request May 18, 2018

[cxxmodules] Enable 17 tests and exclude 2 tests #2040

Merged

[cxxmodules] Fix failing runtime_cxxmodules tests by preloading modules #1814

[cxxmodules] Fix failing runtime_cxxmodules tests by preloading modules #1814

Conversation

yamaguchi1024 commented Apr 3, 2018 • edited

yamaguchi1024 commented Apr 3, 2018

vgvassilev Apr 3, 2018

Choose a reason for hiding this comment

vgvassilev Apr 3, 2018

Choose a reason for hiding this comment

vgvassilev Apr 3, 2018

Choose a reason for hiding this comment

vgvassilev commented Apr 3, 2018

vgvassilev commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

Errors:

Failing tests:

phsft-bot commented Apr 3, 2018

yamaguchi1024 commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

Errors:

Failing tests:

phsft-bot commented Apr 3, 2018

yamaguchi1024 commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

phsft-bot commented Apr 3, 2018

Errors:

Failing tests:

yamaguchi1024 commented Apr 4, 2018

phsft-bot commented Apr 4, 2018

phsft-bot commented Apr 4, 2018

Errors:

Failing tests:

phsft-bot commented Apr 4, 2018

yamaguchi1024 commented Apr 4, 2018

phsft-bot commented Apr 4, 2018

yamaguchi1024 commented Apr 4, 2018

phsft-bot commented Apr 4, 2018

yamaguchi1024 commented Apr 4, 2018

phsft-bot commented May 17, 2018

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Warnings:

Failing tests:

phsft-bot commented May 17, 2018

Warnings:

Failing tests:

phsft-bot commented May 17, 2018

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

phsft-bot commented May 17, 2018

Failing tests:

yamaguchi1024 commented Apr 3, 2018 •

edited

pcanal commented May 21, 2018 •

edited