[REG 2.067.0] Issue 14431 - huge slowdown of compilation speed #4944
Conversation
src/struct.c
Outdated
There was a problem hiding this comment.
Makes sense b/c non-root instances no longer run semantic3.
In the longer run the TypeInfo should only be generated where the struct is instantiated.
In the pull request dlang#4384, all instance has been changed to invoke semantic3(). It was for the link-failure issue in specific case, but it was too excessive. 1. Semantic analysis strategy for template instances: We cannot determine which instance does not need to be placed in object file until semantic analysis completed. Therefore, for all templates instantiated in root module, compiler should invoke their semantic3 -- regardless of whether those are also instantiated in non-root module. If a template is _only_ instantiated in non-root module, we can elide its semantic3 (and for the compilation speed we should do that). 2. Code generation strategy for template instances: If a template is instantiated in non-root module, compiler usually does not have to put it in object file. But if a template is instantiated in both of root and non-root modules which mutually import each other, it needs to placed in objfile.
…th separate compilation
|
Unfortunately this causes a linker error with Higgs' test. I'll try to provide more details, but it seems that some phobos unittest (std.uni) gets compiled into the binary but some templates instantiated by it are not. Might be the |
|
Here is code from Higgs that instantiates the right templates to trigger the issue. module bug;
void foo()
{
import std.regex;
enum hexRegex = ctRegex!("");
}
void main() {}
The problem itself seems to come from dbgVerifySorted. |
|
@MartinNowak If a source file is conditionally compiled from two places by using different conditions ( OT: In #4626, I had tried to reduce such the link-failure problem for instantiated code... |
|
From a practical standpoint we cannot do something that breaks programs using ctRegex though. The problem seems to be a bit more complex. dbgVerifySorted actually does instantiate stride, it's stride.Result.moveAt instantiating std.range.moveAt which is a problem here. For some reason no code is generated for the latter. |
|
It's not so complex. When we try to speed up compilation by avoiding more codegen for the instances, it's equivalent that we try to use more many pre-compiled code in the library file. In there, an implicit assumption exists: "the library and the currently compiled code are analyzed under the same conditional compilation". So, this is unavoidable. By taking the compilation speed, we will have to strictly follow the rule of conditional compilation. |
That is understood, but not exactly the problem we're seeing here.
Both stride and moveAt are only present b/c the non-root module was analyzed with Taking into account the possibly different attributes inferred we apparently have 2 solutions. Generating all instances by default or only linking code that was compiled with the same arguments. The latter required debug/unittest/release phobos libraries or careful avoidance of differences. |
CTFE will invoke semantic3 pass for most of the children of CodepointSet. Therefore currently, compiler can know that SortedRange is its real children. In other words, for the compilation speed, compiler maximally uses the assumption of the conditional compilation (== pre-compiled library and user code are compiled under the same condition). If we adjust the balance of compile speed and link-ability, it should go into future enhancement. |
|
Interestingly enough, we only generate code for stride b/c of this "hack" https://github.com/D-Programming-Language/dmd/blob/97449e602b346f2dd46a06f74cec74ce3e82d173/src/dtemplate.d#L6329. |
|
@MartinNowak Ah, I finally got the issue you've mentioned. Today, when |
|
Please give me some days. I'll start to work for that. |
9rnsr
force-pushed
the
fix14431
branch
8 times, most recently
from
68bd9a5 to
83b5428
Compare
…nd update comments
Even if a struct is defined as non-root symbol, some built-in operations request its TypeInfo. For those, today TypeInfo_Struct is generated in COMDAT. On the other hand, TypeInfoDeclaration is always generated when the TypeInfo is requested, even if the code exists in speculative scope. By that, some unneeded TypeInfo objects had excessively generated. If a TypeInfo is not actually used, we should stop its generation. To satisfy two requirements, I added "speculative TypeInfo" concept associated with template instances. By this change, when a TypeInfo is *requested*, the codegen pass still visit it always. But if the corresponding type is instantiated struct or its derived, and is definitely speculative, the TypeInfo code also won't be emit into object file.
1. When a non-root function is succeeded to expand for inlining, the additional TypeInfo requests should be handled properly. 2. When a function's semantic3 pass will get called from `canInline`, it would generate more speculative instances when -debug/-unittest is specified. If the additionally instantiated function succeeds to expand, it should be changed to non-speculative, because its code is reachable from the final executable.
Issue 14541 - "duplicate COMDAT" linker error with the template forward reference in Tuple.opAssign
|
OK, ready to review.
|
|
I tested this with a bunch of the most downloaded packages and couldn't find any issue. |
|
Auto-merge toggled on |
[REG 2.067.0] Issue 14431 - huge slowdown of compilation speed
|
Unfortunately I hit a linking issue when building chmgen. import std.file;
struct Only(T)
{
private T _val;
}
auto only(V)(V v)
{
return Only!V(v);
}
static struct Chain(R...)
{
R source;
}
auto chain(R...)(R rs)
{
return Chain!R(rs);
}
void main()
{
string docRoot;
chain(
dirEntries(docRoot , "*.*" , SpanMode.shallow),
only(docRoot)
);
}Seems like TypeInfo for |
|
Issue 14985 - [REG2.068.1-b1] Link failure for const TypeInfo of speculative instantiated struct |
|
Seems like we have another regression. |
Even if a function is expanded by inlining, the function itself don't have to put in object file. In short, adjusting `ti.minst` in `inline.d` is not correct. The unspeculation in `expandInline` was added in the PR dlang#4944, but sadly it was just a hack and inherently unneeded. The changes in `TemplateInstance.appendToModuleMember` and `needsCodegen` those were added since 2.068.1-b1, now correctly handle codegen of nested template instances.
|
This pull request introduced a regression: |
|
This pull request introduced a regression: |
4 participants
This is same with #4784, excepting the base branch is 'stable'.
https://issues.dlang.org/show_bug.cgi?id=14431
In the pull request #4384, all instance has been changed to invoke
semantic3(). It was for the link-failure issue in specific case, but it
was too excessive.
We cannot determine which instance does not need to be placed in object
file until semantic analysis completed. Therefore, for all templates
instantiated in root module, compiler should invoke their semantic3 --
regardless of whether those are also instantiated in non-root module. If
a template is only instantiated in non-root module, we can elide its
semantic3 (and for the compilation speed we should do that).
If a template is instantiated in non-root module, compiler usually does
not have to put it in object file. But if a template is instantiated in
both of root and non-root modules which mutually import each other, it
needs to placed in objfile.
By fixing excessive semantic3 for template instances, we can also fix few regressions which come from the same root.