-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[cxxmodule] Don't declare fwd decls from rootmaps with modules #2592
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
Conversation
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
|
|
@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on |
|
Starting build on |
|
We should skip loading rootmap files only if we have a pcm file. |
a002453 to
798b7f7
Compare
|
Starting build on |
|
We should not read decls from rootmap as they are already in pcm, no? |
|
Build failed on fedora28/native. Warnings:
|
|
The current patch essentially disabled rootmap files if we built with -Druntime_cxxmodules=On. That would break experiments which will still use the incremental approach where ROOT will rely on pcm files and their libraries on rootmaps. We should not read the rootmap files which have a preloaded module. |
|
Doesn't the precompiled module have to be generated by rootcling/genreflex to be useful for us? Consequently shouldn't rootcling either generated a full rootmap or a short-rootmap and a precompiled module? |
|
I think Vassil's point is that there is still a little bit of un-modularized part in ROOT which is relying on rootmap files, and we should skip rootmap files only when we know that we have pcm. right? |
798b7f7 to
efceb6b
Compare
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 1 more |
That's fine ... however I thought 'some' part of the rootmap file was still needed ... |
|
I'm just skipping forward decls from rootmaps, not all of them. |
So if I understand correctly we still need the rootmap file even for the one with a precompiled module, or am I missing something? |
|
Yes, I'm trying not to use rootmap files at all (#2380), but there is still things to do such as adding support of modules to ACLiC. (This is unrelated to this patch, though) |
efceb6b to
9231afa
Compare
|
@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on |
|
Starting build on |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 25 more |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_TYPE=Debug |
|
Starting build on |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD_TYPE=Release |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 25 more |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_TYPE=RelWithDebInfo |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 18 more |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_TYPE=RelWithDebInfo -Dccache=Off |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 18 more |
869abe7 to
26e3ed3
Compare
|
Starting build on |
Fwd decls from rootmap files were injected to ROOT at the startup time. However with modules we don't need any foward decls as we're preloading all modules. Our future plan is to remove rootmap files as a whole, but we still need to support more features (see 2380). So for now we can remove what we are not using. This improves startup memory by 10 mbytes.
|
Build failed on fedora28/native. Warnings:
And 18 more |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_TYPE=Release -Dccache=ON |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 18 more |
|
@phsft-bot build just on fedora28/native with flags -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_TYPE=Release -Dccache=ON |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 18 more |
|
@phsft-bot build! |
|
Starting build on |
|
Build failed on fedora28/native. Warnings:
And 29 more |
|
@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on |
|
Starting build on |
Fwd decls from rootmap files were injected to ROOT at the startup time.
However with modules we don't need any foward decls as we're preloading
all modules.
Our future plan is to remove rootmap files as a whole, but we still need
to support more features (see 2380). So for now we can remove what we
are not using.
*edit This patch also contains adding fHasCxxModule as a private member
This improves startup memory by 10 mbytes.