[cxxmodules] Only load modules if one was generated #1511

Teemperor · 2018-01-15T10:07:55Z

If we generate a dictionary with a C++ module with rootcling we
should write in the payload that we generated a C++ module.
This way we can actually print a reliable warning when we fail
to load the C++ module during runtime. Without this we can only
speculate if a missing C++ module is intentional.

phsft-bot · 2018-01-15T10:08:18Z

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

Teemperor · 2018-01-15T10:08:46Z

This commit is part of #1394 and I want to get it in while the rest of the other PR is being worked on.

phsft-bot · 2018-01-15T11:01:03Z

Build failed on slc6/gcc62.
See console output.

Errors:

ERROR: Timeout after 10 minutes
ERROR: Error fetching remote repo 'origin'
stderr: error: RPC failed; result=18, HTTP code = 200
error: fetch-pack died of signal 15
ERROR: Error fetching remote repo 'origin'

phsft-bot · 2018-01-15T11:14:05Z

Build failed on mac1012/native.
See console output.

Errors:

ERROR: Error fetching remote repo 'origin'
error: RPC failed; curl 18 transfer closed with outstanding read data remaining
ERROR: Error fetching remote repo 'origin'

Axel-Naumann

If a cxx module was generated but cannot be loaded - is that a severe issue? Or is the module optional?

If it's optional, should we just try to load it and fail gracefully if it's not installed? But maybe that's the cause of a perf issue that's hard to track down?

Anyway - thank you so much for providing this patch!

Axel-Naumann · 2018-01-16T15:19:58Z

core/dictgen/src/TModuleGenerator.cxx

                              "      TROOT::RegisterModule(\"" << GetDemangledDictionaryName() << "\",\n"
                              "        headers, includePaths, payloadCode, fwdDeclCode,\n"
-                              "        TriggerDictionaryInitialization_" << GetDictionaryName() << "_Impl, " << fwdDeclnArgsToKeepString << ", classesHeaders);\n"
+                              "        TriggerDictionaryInitialization_" << GetDictionaryName() << "_Impl, " << fwdDeclnArgsToKeepString << ", classesHeaders, " << (fCI->getLangOpts().Modules ? "/*has C++ module*/true" : "/*has no C++ module*/false") << ");\n"


Can you make that , /*hasCxxModule*/" << (fCI->getLangOpts().Modules ? "true" : "false")?

/*has no C++ module*/ false sounds like a double negation...

vgvassilev · 2018-01-16T20:19:03Z

@phsft-bot build!

phsft-bot · 2018-01-16T20:19:11Z

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

If we generate a dictionary with a C++ module with rootcling we should write in the payload that we generated a C++ module. This way we can actually print a *reliable* warning when we fail to load the C++ module during runtime. Without this we can only speculate if a missing C++ module is intentional.

phsft-bot · 2018-01-17T10:17:23Z

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

Teemperor · 2018-01-17T10:19:37Z

@Axel-Naumann Shouldn't matter (beside performance) at the moment if a module can't be loaded. So far we only print a message to inform that our loading setup is still wrong and then continue.

Teemperor · 2018-01-19T07:53:11Z

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

phsft-bot · 2018-01-19T07:53:18Z

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

Teemperor · 2018-01-19T12:03:48Z

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

phsft-bot · 2018-01-19T12:03:58Z

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

Teemperor · 2018-01-19T14:50:19Z

@phsft-bot build just on centos7/gcc7 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

phsft-bot · 2018-01-19T14:50:27Z

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

phsft-bot · 2018-01-19T16:26:46Z

Build failed on centos7/gcc7.
See console output.

Errors:

298/1655 Test New Methods #192: tutorial-dataframe-tdf013_InspectAnalysis .........................................................***Failed Error regular expression found in output. Regex=[: error:] 2.13 sec
546/1655 Test Add support for gtest if testing is enabled. #451: tutorial-math-quasirandom .........................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.42 sec
576/1655 Test Faster default/copy construction of TObjects #482: tutorial-multicore-mtbb201_parallelHistoFill ......................................................***Failed Error regular expression found in output. Regex=[: error:] 5.59 sec
689/1655 Test [TDF] Fix Snapshot not handling multiple input files correctly #601: tutorial-roostats-Zbi_Zgamma ......................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.59 sec
702/1655 Test [Math] GSLMultiRootFinder: Fix return pointer to local address #613: tutorial-roostats-rs_numbercountingutils ..........................................................***Failed Error regular expression found in output. Regex=[: error:] 1.73 sec
736/1655 Test Make cling build again on Windows (following the recent changes). To … #653: tutorial-tmva-keras-LaunchClasGUI .................................................................***Failed Error regular expression found in output. Regex=[: error:] 1.21 sec
738/1655 Test [WIP] Vectorization of TMath #655: tutorial-tmva-keras-LaunchRegGUI ..................................................................***Failed Error regular expression found in output. Regex=[: error:] 2.42 sec
739/1655 Test Fix warnings on no-imt builds #654: tutorial-tmva-keras-LaunchMultiGUI ................................................................***Failed Error regular expression found in output. Regex=[: error:] 3.76 sec
781/1655 Test TH1 KolmogorovTest infinite loop #705: tutorial-dataframe-tdf002_dataModel-py ............................................................***Failed Error regular expression found in output. Regex=[: error:] 2.02 sec

Failing tests:

And 25 more

Teemperor · 2018-01-22T11:20:35Z

Ok, Jenkins passed without modules and with modules we have no regressions it seems, so merging.

Teemperor assigned vgvassilev Jan 15, 2018

Teemperor requested review from Axel-Naumann and vgvassilev January 15, 2018 10:07

Teemperor requested a review from pcanal as a code owner January 15, 2018 10:07

vgvassilev approved these changes Jan 15, 2018

View reviewed changes

Axel-Naumann requested changes Jan 16, 2018

View reviewed changes

Teemperor force-pushed the OnlyLoadExistingModules branch from 5d835cc to 4900610 Compare January 17, 2018 10:17

Teemperor merged commit 48fc07c into root-project:master Jan 22, 2018

Teemperor deleted the OnlyLoadExistingModules branch January 22, 2018 11:20

[cxxmodules] Only load modules if one was generated #1511

[cxxmodules] Only load modules if one was generated #1511

Uh oh!

Conversation

Teemperor commented Jan 15, 2018

Uh oh!

phsft-bot commented Jan 15, 2018

Uh oh!

Teemperor commented Jan 15, 2018

Uh oh!

phsft-bot commented Jan 15, 2018

Errors:

Uh oh!

phsft-bot commented Jan 15, 2018

Errors:

Uh oh!

Axel-Naumann left a comment

Choose a reason for hiding this comment

Uh oh!

Axel-Naumann Jan 16, 2018

Choose a reason for hiding this comment

Uh oh!

Teemperor Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

vgvassilev commented Jan 16, 2018

Uh oh!

phsft-bot commented Jan 16, 2018

Uh oh!

phsft-bot commented Jan 17, 2018

Uh oh!

Teemperor commented Jan 17, 2018

Uh oh!

Teemperor commented Jan 19, 2018

Uh oh!

phsft-bot commented Jan 19, 2018

Uh oh!

Teemperor commented Jan 19, 2018

Uh oh!

phsft-bot commented Jan 19, 2018

Uh oh!

Teemperor commented Jan 19, 2018

Uh oh!

phsft-bot commented Jan 19, 2018

Uh oh!

phsft-bot commented Jan 19, 2018

Errors:

Failing tests:

Uh oh!

Teemperor commented Jan 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants