Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Axel-Naumann
Copy link
Member

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.

@Axel-Naumann Axel-Naumann self-assigned this Apr 27, 2021
@phsft-bot
Copy link
Collaborator

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

1 similar comment
@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-04-27T16:50:46.080Z] FAILED: core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o
  • [2021-04-27T16:50:46.338Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:27: error: cannot convert ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’} to ‘const char*’

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-04-27T16:51:21.339Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:27: error: cannot convert ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’} to ‘const char*’

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-04-27T16:51:56.108Z] FAILED: /usr/bin/ccache /usr/bin/c++ -I/mnt/build/workspace/root-pullrequests-build/root/interpreter/cling/include -I/mnt/build/workspace/root-pullrequests-build/root/core/metacling/res -I/mnt/build/workspace/root-pullrequests-build/root/core/clingutils/res -I/mnt/build/workspace/root-pullrequests-build/root/core/foundation/res -I/mnt/build/workspace/root-pullrequests-build/root/core/zip/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/clib/res -I/mnt/build/workspace/root-pullrequests-build/root/core/base/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/meta/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/clib/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/foundation/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/thread/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/cont/inc -I/mnt/build/workspace/root-pullrequests-build/root/core/gui/inc -I/mnt/build/workspace/root-pullrequests-build/root/io/io/inc -Iginclude -isystem /mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/tools/clang/include -isystem interpreter/llvm/src/tools/clang/include -isystem /mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include -isystem interpreter/llvm/src/include -fdiagnostics-color=always -std=c++11 -pipe -Wshadow -Wall -W -Woverloaded-virtual -fsigned-char -pthread -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fvisibility=hidden -fno-strict-aliasing -Wwrite-strings -Wno-shadow -Wno-unused-parameter -Wno-deprecated-declarations -O3 -fPIC -std=c++11 -MD -MT core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o -MF core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o.d -o core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o -c /mnt/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx
  • [2021-04-27T16:51:56.108Z] /mnt/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:31: error: no matching function for call to ‘TSystem::Load(std::__cxx11::string&)’

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-27T16:54:20.077Z] /data/sftnight/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:31: error: no matching function for call to ‘TSystem::Load(std::__cxx11::string&)’

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-04-27T17:04:21.074Z] FAILED: core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o
  • [2021-04-27T17:04:21.341Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:27: error: no viable conversion from 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >') to 'const char *'

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-04-27T17:03:41.617Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:31: error: no matching function for call to ‘TSystem::Load(std::__cxx11::string&)’

@phsft-bot
Copy link
Collaborator

Build failed on mac1014/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-04-27T17:22:21.283Z] FAILED: core/metacling/src/CMakeFiles/MetaCling.dir/TCling.cxx.o
  • [2021-04-27T17:22:21.853Z] /Volumes/HD2/build/workspace/root-pullrequests-build/root/core/metacling/src/TCling.cxx:7209:27: error: no viable conversion from 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >') to 'const char *'

@phsft-bot
Copy link
Collaborator

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

core/base/src/TSystem.cxx Outdated Show resolved Hide resolved
@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Warnings:

  • [2021-04-27T18:55:01.262Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/src/TSystem.cxx:3326:21: warning: unused variable ‘onetime’ [-Wunused-variable]

Failing tests:

@phsft-bot
Copy link
Collaborator

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

Warnings:

  • [2021-04-27T18:55:03.305Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/src/TSystem.cxx:3326:21: warning: unused variable ‘onetime’ [-Wunused-variable]

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2021-04-27T18:53:05.118Z] /data/sftnight/workspace/root-pullrequests-build/root/core/base/src/TSystem.cxx:3326:21: warning: unused variable ‘onetime’ [-Wunused-variable]

Failing tests:

@phsft-bot
Copy link
Collaborator

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

@Axel-Naumann
Copy link
Member Author

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

@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@Axel-Naumann Axel-Naumann marked this pull request as ready for review November 2, 2021 16:14
@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2021-11-02T17:22:09.548Z] CMake Error at CMakeLists.txt:141 (message):
  • [2021-11-02T17:22:09.548Z] CMake Error at /Users/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1128 (message):

@Axel-Naumann
Copy link
Member Author

@phsft-bot build just on mac11.0/cxx17

@phsft-bot
Copy link
Collaborator

Starting build on mac11.0/cxx17
How to customize builds

@Axel-Naumann Axel-Naumann force-pushed the ACLiC-do-not-walk-all-libs-for-U-syms branch from eb7903c to 8c1ed58 Compare November 3, 2021 15:15
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@Axel-Naumann Axel-Naumann force-pushed the ACLiC-do-not-walk-all-libs-for-U-syms branch from 8c1ed58 to c08b2da Compare November 3, 2021 18:04
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

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

Failing tests:

@phsft-bot
Copy link
Collaborator

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

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.
@Axel-Naumann Axel-Naumann force-pushed the ACLiC-do-not-walk-all-libs-for-U-syms branch from c08b2da to ed10012 Compare November 4, 2021 07:29
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

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

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);
Copy link
Member

@pcanal pcanal Nov 5, 2021

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

Copy link
Member

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

Copy link
Member

@pcanal pcanal left a 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.

@ferdymercury
Copy link
Collaborator

Other speedup related issues:
#14287
#7123

Copy link

Test Results

    12 files      12 suites   3d 0h 55m 25s ⏱️
 2 697 tests  2 693 ✅ 0 💤 4 ❌
30 364 runs  30 360 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit ed10012.

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

Successfully merging this pull request may close these issues.

6 participants