Skip to content

Commit

Permalink
fix Issue 14431 - huge slowdown of compilation speed
Browse files Browse the repository at this point in the history
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.

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.
  • Loading branch information
9rnsr committed Aug 24, 2015
1 parent 45c115b commit 6c14fad
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 79 deletions.
8 changes: 6 additions & 2 deletions src/struct.c
Expand Up @@ -100,8 +100,12 @@ void semanticTypeInfo(Scope *sc, Type *t)

// If the struct is in a non-root module, run semantic3 to get
// correct symbols for the member function.
// Note that, all instantiated symbols will run semantic3.
if (sd->inNonRoot())
if (TemplateInstance *ti = sd->isInstantiated())
{
if (ti->minst && !ti->minst->isRoot())
Module::addDeferredSemantic3(sd);
}
else if (sd->inNonRoot())
{
//printf("deferred sem3 for TypeInfo - sd = %s, inNonRoot = %d\n", sd->toChars(), sd->inNonRoot());
Module::addDeferredSemantic3(sd);
Expand Down
180 changes: 107 additions & 73 deletions src/template.c
Expand Up @@ -5942,22 +5942,35 @@ void TemplateInstance::semantic(Scope *sc, Expressions *fargs)
this->tnext = inst->tnext;
inst->tnext = this;

// If the first instantiation was in speculative context, but this is not:
if (tinst && !inst->tinst && !inst->minst)
if (minst && minst->isRoot() && !(inst->minst && inst->minst->isRoot()))
{
// Reconnect the chain if this instantiation is not in speculative context.
/* Swap the position of 'inst' and 'this' in the instantiation graph.
* Then, the primary instance `inst` will be changed to a root instance.
*
* Before:
* non-root -> A!() -> B!()[inst] -> C!()
* |
* root -> D!() -> B!()[this]
*
* After:
* non-root -> A!() -> B!()[this]
* |
* root -> D!() -> B!()[inst] -> C!()
*/
Module *mi = minst;
TemplateInstance *ti = tinst;
while (ti && ti != inst)
ti = ti->tinst;
if (ti != inst) // Bugzilla 13379: Prevent circular chain
inst->tinst = tinst;
}
minst = inst->minst;
tinst = inst->tinst;
inst->minst = mi;
inst->tinst = ti;

// If the first instantiation was speculative, but this is not:
if (!inst->minst)
{
// Mark it is a non-speculative instantiation.
inst->minst = minst;
if (minst) // if inst was not speculative
{
/* Add 'inst' once again to the root module members[], then the
* instance members will get codegen chances.
*/
inst->appendToModuleMember();
}
}

#if LOG
Expand All @@ -5979,66 +5992,10 @@ void TemplateInstance::semantic(Scope *sc, Expressions *fargs)

//getIdent();

// Add 'this' to the enclosing scope's members[] so the semantic routines
// will get called on the instance members. Store the place we added it to
// in target_symbol_list(_idx) so we can remove it later if we encounter
// an error.
#if 1
Dsymbols *target_symbol_list;
size_t target_symbol_list_idx = 0;
//if (sc->scopesym) printf("3: sc is %s %s\n", sc->scopesym->kind(), sc->scopesym->toChars());
if (0 && !tinst && sc->scopesym && sc->scopesym->members)
{
/* A module can have explicit template instance and its alias
* in module scope (e,g, `alias Base64Impl!('+', '/') Base64;`).
* When the module is just imported, normally compiler can assume that
* its instantiated code would be contained in the separately compiled
* obj/lib file (e.g. phobos.lib).
* Bugzilla 2644: However, if the template is instantiated in both
* modules of root and non-root, compiler should generate its objcode.
* Therefore, always conservatively insert this instance to the member of
* a root module, then calculate the necessity by TemplateInstance::needsCodegen().
*/
//if (sc->scopesym->isModule())
// printf("module level instance %s\n", toChars());

//printf("\t1: adding to %s %s\n", sc->scopesym->kind(), sc->scopesym->toChars());
target_symbol_list = sc->scopesym->members;
}
else
{
Dsymbol *s = enclosing ? enclosing : tempdecl->parent;
while (s && !s->isModule())
s = s->toParent2();
assert(s);
Module *m = (Module *)s;
if (!m->isRoot())
m = m->importedFrom;

//printf("\t2: adding to module %s instead of module %s\n", m->toChars(), sc->module->toChars());
target_symbol_list = m->members;

/* Defer semantic3 running in order to avoid mutual forward reference.
* See test/runnable/test10736.d
*/
if (m->semanticRun >= PASSsemantic3done)
Module::addDeferredSemantic3(this);
}
for (size_t i = 0; 1; i++)
{
if (i == target_symbol_list->dim)
{
target_symbol_list_idx = i;
target_symbol_list->push(this);
break;
}
if (this == (*target_symbol_list)[i]) // if already in Array
{
target_symbol_list = NULL;
break;
}
}
#endif
// Store the place we added it to in target_symbol_list(_idx) so we can
// remove it later if we encounter an error.
Dsymbols *target_symbol_list = appendToModuleMember();
size_t target_symbol_list_idx = target_symbol_list ? target_symbol_list->dim - 1 : 0;

// Copy the syntax trees from the TemplateDeclaration
members = Dsymbol::arraySyntaxCopy(tempdecl->members);
Expand Down Expand Up @@ -7288,6 +7245,83 @@ bool TemplateInstance::hasNestedArgs(Objects *args, bool isstatic)
return nested != 0;
}

/*****************************************
* Append 'this' to the enclosing module members[] so the semantic routines
* will get called on the instance members.
*/
Dsymbols *TemplateInstance::appendToModuleMember()
{
Module *md = tempdecl->getModule();

Module *mi = minst;
if (mi)
{
if (mi->isRoot())
{
/* If both modules mi and md where the template is declared are
* roots, instead use md to store the generated code in it.
*/
if (md && md->isRoot())
mi = md;
}
else
{
/* A module can have explicit template instance and its alias
* in module scope (e,g, `alias Base64Impl!('+', '/') Base64;`).
* When the module is just imported, normally compiler can assume that
* its instantiated code would be contained in the separately compiled
* obj/lib file (e.g. phobos.lib).
*/
/*
* Bugzilla 2644: However, if the template is instantiated in both
* modules of root and non-root, compiler should generate its objcode.
* Therefore, always conservatively insert this instance to the member of
* a root module, then calculate the necessity by TemplateInstance::needsCodegen().
*/
}
//printf("\t1: adding to %s\n", mi->toPrettyChars());
}
else
{
// 'this' is speculative instance
if (md && md->isRoot())
{
mi = md;
}
else
{
// select arbitrary root module
Dsymbol *s = enclosing ? enclosing : tempdecl->parent;
while (s && !s->isModule())
s = s->toParent2();
assert(s);
mi = (Module *)s;
if (!mi->isRoot())
mi = mi->importedFrom;
}
assert(mi->isRoot());
//printf("\t2: adding to module %s instead of module %s\n", mi->toPrettyChars(), sc->module->toPrettyChars());
}

Dsymbols *a = mi->members;
for (size_t i = 0; 1; i++)
{
if (i == a->dim)
{
a->push(this);
if (mi->semanticRun >= PASSsemantic3done && mi->isRoot())
Module::addDeferredSemantic3(this);
break;
}
if (this == (*a)[i]) // if already in Array
{
a = NULL;
break;
}
}
return a;
}

/****************************************
* This instance needs an identifier for name mangling purposes.
* Create one by taking the template declaration name and adding
Expand Down
1 change: 1 addition & 0 deletions src/template.h
Expand Up @@ -351,6 +351,7 @@ class TemplateInstance : public ScopeDsymbol
bool findBestMatch(Scope *sc, Expressions *fargs);
bool needsTypeInference(Scope *sc, int flag = 0);
bool hasNestedArgs(Objects *tiargs, bool isstatic);
Dsymbols *appendToModuleMember();
void declareParameters(Scope *sc);
Identifier *genIdent(Objects *args);
void expandMembers(Scope *sc);
Expand Down
5 changes: 4 additions & 1 deletion test/runnable/extra-files/test14198.d
Expand Up @@ -13,8 +13,11 @@ struct S
to!string(false);
// [1] to!string(bool src) should be deduced to pure @safe, and the function will be mangled to:
// --> _D8std141984conv11__T2toTAyaZ9__T2toTbZ2toFNaNbNiNfbZAya
// [2] its object code should be stored in the library file, because it's instantiated in std14188.uni:
// [2] its object code would be stored in the library file, because it's instantiated in std14188.uni:
// --> FormatSpec!char --> to!string(bool src) in FormatSpec!char.toString()
// But semanti3 of FormatSpec!char.toString() won't get called from this module compilation,
// so the instantiaion is invisible.
// Then, the object code is also stored in test14198.obj, and the link will succeed.
}
else
static assert(0);
Expand Down
6 changes: 3 additions & 3 deletions test/runnable/link14198b.sh
Expand Up @@ -13,13 +13,13 @@ else
fi
libname=${dir}${SEP}lib14198b${LIBEXT}

# Do link failure without library file.
# Do not link failure even without library file.

$DMD -m${MODEL} -I${src} -of${dir}${SEP}test14198b${EXE} ${src}${SEP}test14198.d > ${output_file} 2>&1
grep -q "_D8std141984conv11__T2toTAyaZ9__T2toTbZ2toFNaNbNiNfbZAya" ${output_file} || exit 1
grep -q "_D8std141984conv11__T2toTAyaZ9__T2toTbZ2toFNaNbNiNfbZAya" ${output_file} && exit 1

$DMD -m${MODEL} -I${src} -of${dir}${SEP}test14198b${EXE} -version=bug14198 ${src}${SEP}test14198.d > ${output_file} 2>&1
grep -q "_D8std141984conv11__T2toTAyaZ9__T2toTbZ2toFNaNbNiNfbZAya" ${output_file} || exit 1
grep -q "_D8std141984conv11__T2toTAyaZ9__T2toTbZ2toFNaNbNiNfbZAya" ${output_file} && exit 1

rm ${dir}/{test14198b${OBJ},test14198b${EXE}}

Expand Down

0 comments on commit 6c14fad

Please sign in to comment.