-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
[core] ACLiC: speed up building / loading: #8017
base: master
Are you sure you want to change the base?
[core] ACLiC: speed up building / loading: #8017
Conversation
Starting build on |
1 similar comment
Starting build on |
Build failed on ROOT-fedora30/cxx14. Errors:
|
Build failed on ROOT-fedora31/noimt. Errors:
|
Build failed on ROOT-ubuntu16/nortcxxmod. Errors:
|
Build failed on ROOT-performance-centos8-multicore/default. Errors:
|
Build failed on mac11.0/cxx17. Errors:
|
Build failed on ROOT-debian10-i386/cxx14. Errors:
|
Build failed on mac1014/python3. Errors:
|
Starting build on |
Build failed on ROOT-fedora31/noimt. Failing tests:
|
Build failed on ROOT-fedora30/cxx14. Failing tests:
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests:
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests:
|
Build failed on ROOT-debian10-i386/cxx14. Failing tests:
|
Starting build on |
Build failed on ROOT-fedora31/noimt. Warnings:
Failing tests:
|
Build failed on ROOT-fedora30/cxx14. Warnings:
Failing tests:
|
Build failed on ROOT-performance-centos8-multicore/default. Warnings:
Failing tests:
|
Starting build on |
I'll clean up git history once the tests pass - there's root-project/roottest#699 and the very different projectroot.core.metacling.test.gtest_core_metacling_test_TClingTest |
Build failed on ROOT-fedora31/noimt. Failing tests:
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests:
|
Starting build on |
Build failed on mac11.0/cxx17. Errors:
|
@phsft-bot build just on mac11.0/cxx17 |
Starting build on |
eb7903c
to
8c1ed58
Compare
Starting build on |
Build failed on mac11.0/cxx17. Failing tests: |
8c1ed58
to
c08b2da
Compare
Starting build on |
Build failed on mac1014/python3. Failing tests: |
Build failed on mac11.0/cxx17. Failing tests: |
When loading an existing ACLiC library, and if we expect it to contain the dependencies (explicit linking), just load it, instead of trying to (re-)determine its dependencies from its undefined symbols: the outcome should be just the library dependencies we expect the library to know already. For the (rare) non-explicit-linking case, continue to find and load all dependent libraries. Way slower, but also way rarer.
c08b2da
to
ed10012
Compare
Starting build on |
Build failed on mac11.0/cxx17. Failing tests: |
@@ -3374,8 +3380,6 @@ int TSystem::CompileMacro(const char *filename, Option_t *opt, | |||
if (!keep) k->SetBit(kMustCleanup); | |||
fCompiled->Add(k); | |||
|
|||
gInterpreter->GetSharedLibDeps(library); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call is still needed for the not-explicitly-linked case. It has the side effect of load the dependencies from the rootmap files so that the subsequent LoadLibrary can use that information. See commit 8fdd5c3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related we should probably deprecate the option to disable explicit linking in ACLiC (it is already removed from the main build). Eitherway removing this line probably require to add (or check we already have) a test for the problem solved by commit 8fdd5c3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved but need one more correction.
Test Results 12 files 12 suites 3d 0h 55m 25s ⏱️ For more details on these failures, see this check. Results for commit ed10012. |
Instead of looking for libraries resolving each unresolved symbol,
just load each lib resolving symbols: this will allow subsequent
missing symbols to be resolved from the loaded library quickly,
without touching disk.
When loading an existing ACLiC library, and if we expect it
to contain the dependencies (explicit linking), just load itt,
instead of trying to (re-)determine its dependencies from its
undefined symbols: the outcome should be just the library
dependencies we expect the library to know already.