Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

TypeInfo_Struct: Switch to stored mangled name & demangle lazily (with per-thread cache) #3527

Merged
merged 7 commits into from
Aug 1, 2021
22 changes: 20 additions & 2 deletions .cirrus.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,31 @@
common_steps_template: &COMMON_STEPS_TEMPLATE
set_repo_branch_env_var_script: |
set -uexo pipefail
if [ -z ${CIRRUS_PR+x} ]; then
# not a PR
REPO_BRANCH="$CIRRUS_BRANCH"
elif [[ ! "$CIRRUS_BRANCH" =~ ^pull/ ]]; then
# PR originating from the official dlang repo
REPO_BRANCH="$CIRRUS_BRANCH"
else
# PR from a fork
REPO_BRANCH="$CIRRUS_BASE_BRANCH"
fi
echo "REPO_BRANCH=$REPO_BRANCH" >> $CIRRUS_ENV
clone_dmd_script: |
set -uexo pipefail
git clone --branch "${CIRRUS_BASE_BRANCH:-$CIRRUS_BRANCH}" --depth 1 https://github.com/dlang/dmd.git ../dmd
DMD_BRANCH="$REPO_BRANCH"
if [ "$DMD_BRANCH" != master ] && [ "$DMD_BRANCH" != stable ] &&
! git ls-remote --exit-code --heads "https://github.com/dlang/dmd.git" "$DMD_BRANCH" > /dev/null; then
DMD_BRANCH="master"
fi
git clone --branch "$DMD_BRANCH" --depth 1 https://github.com/dlang/dmd.git ../dmd
install_prerequisites_script: cd ../dmd && ./cirrusci.sh
install_host_compiler_script: cd ../dmd && ./ci.sh install_host_compiler
setup_repos_script: |
set -uexo pipefail
ln -s $CIRRUS_WORKING_DIR ../druntime
cd ../dmd && ./ci.sh setup_repos "${CIRRUS_BASE_BRANCH:-$CIRRUS_BRANCH}"
cd ../dmd && ./ci.sh setup_repos "$REPO_BRANCH"
build_script: cd ../dmd && ./ci.sh build
test_dmd_script: cd ../dmd && ./ci.sh test_dmd
test_druntime_script: cd ../dmd && ./ci.sh test_druntime
Expand Down
37 changes: 34 additions & 3 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Learn more: https://aka.ms/yaml

variables:
DMD_BRANCH: $[ coalesce(variables['System.PullRequest.TargetBranch'], variables['Build.SourceBranchName'], 'master') ]
VSINSTALLDIR: C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\

jobs:
Expand Down Expand Up @@ -34,7 +33,23 @@ jobs:
sourceFolder: '$(Build.SourcesDirectory)'
contents: '**/*'
TargetFolder: '$(Build.SourcesDirectory)/../druntime'
- script: git clone --branch $(DMD_BRANCH) --depth 1 https://github.com/dlang/dmd.git ../dmd
- bash: |
set -ex
if [ -z ${SYSTEM_PULLREQUEST_TARGETBRANCH+x} ]; then
# no PR
DMD_BRANCH="$BUILD_SOURCEBRANCHNAME"
elif [ $SYSTEM_PULLREQUEST_ISFORK == False ]; then
# PR originating from the official dlang repo
DMD_BRANCH="$SYSTEM_PULLREQUEST_SOURCEBRANCH"
else
# PR from a fork
DMD_BRANCH="$SYSTEM_PULLREQUEST_TARGETBRANCH"
fi
if [ "$DMD_BRANCH" != master ] && [ "$DMD_BRANCH" != stable ] &&
! git ls-remote --exit-code --heads "https://github.com/dlang/dmd.git" "$DMD_BRANCH" > /dev/null; then
DMD_BRANCH="master"
fi
git clone --branch "$DMD_BRANCH" --depth 1 https://github.com/dlang/dmd.git ../dmd
displayName: Clone DMD repo
- script: |
call "%VSINSTALLDIR%\VC\Auxiliary\Build\vcvarsall.bat" %ARCH%
Expand Down Expand Up @@ -72,7 +87,23 @@ jobs:
sourceFolder: '$(Build.SourcesDirectory)'
contents: '**/*'
TargetFolder: '$(Build.SourcesDirectory)/../druntime'
- script: git clone --branch $(DMD_BRANCH) --depth 1 https://github.com/dlang/dmd.git ../dmd
- bash: |
set -ex
if [ -z ${SYSTEM_PULLREQUEST_TARGETBRANCH+x} ]; then
# no PR
DMD_BRANCH="$BUILD_SOURCEBRANCHNAME"
elif [ $SYSTEM_PULLREQUEST_ISFORK == False ]; then
# PR originating from the official dlang repo
DMD_BRANCH="$SYSTEM_PULLREQUEST_SOURCEBRANCH"
else
# PR from a fork
DMD_BRANCH="$SYSTEM_PULLREQUEST_TARGETBRANCH"
fi
if [ "$DMD_BRANCH" != master ] && [ "$DMD_BRANCH" != stable ] &&
! git ls-remote --exit-code --heads "https://github.com/dlang/dmd.git" "$DMD_BRANCH" > /dev/null; then
DMD_BRANCH="master"
fi
git clone --branch "$DMD_BRANCH" --depth 1 https://github.com/dlang/dmd.git ../dmd
displayName: Clone DMD repo
- script: cd ../dmd && sh --login .azure-pipelines/windows-visual-studio.sh
displayName: Download required binaries
Expand Down
14 changes: 14 additions & 0 deletions changelog/UniqueTypeInfoNames.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
TypeInfo names for aggregates are fully qualified and hence unique now

Previously, template arguments weren't fully qualified; they now are,
implying longer names in that case.

`TypeInfo_Struct` instances now store the (potentially significantly shorter)
mangled name only and demangle it lazily on the first `name` or `toString()`
call (with a per-thread cache). So if you only need a unique string per
struct TypeInfo, prefer `mangledName` over computed `name` (non-`@nogc` and
non-`pure`).

**Related breaking change**: `TypeInfo.toString()` isn't `pure` anymore to
account for the `TypeInfo_Struct` demangled name cache.
`TypeInfo_Class.toString()` and others are still `pure`.
2 changes: 1 addition & 1 deletion src/core/internal/gc/impl/conservative/gc.d
Original file line number Diff line number Diff line change
Expand Up @@ -4058,7 +4058,7 @@ string debugTypeName(const(TypeInfo) ti) nothrow
else if (auto ci = cast(TypeInfo_Class)ti)
name = ci.name;
else if (auto si = cast(TypeInfo_Struct)ti)
name = si.name;
name = si.mangledName; // .name() might GC-allocate, avoid deadlock
else if (auto ci = cast(TypeInfo_Const)ti)
static if (__traits(compiles,ci.base)) // different whether compiled with object.di or object.d
return debugTypeName(ci.base);
Expand Down
48 changes: 38 additions & 10 deletions src/object.d
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ struct OffsetTypeInfo
*/
class TypeInfo
{
override string toString() const pure @safe nothrow
override string toString() const @safe nothrow
{
return typeid(this).name;
}
Expand Down Expand Up @@ -665,7 +665,7 @@ class TypeInfo

class TypeInfo_Enum : TypeInfo
{
override string toString() const { return name; }
override string toString() const pure { return name; }

override bool opEquals(Object o)
{
Expand Down Expand Up @@ -977,7 +977,9 @@ class TypeInfo_StaticArray : TypeInfo
import core.internal.string : unsignedToTempString;

char[20] tmpBuff = void;
return value.toString() ~ "[" ~ unsignedToTempString(len, tmpBuff) ~ "]";
const lenString = unsignedToTempString(len, tmpBuff);

return (() @trusted => cast(string) (value.toString() ~ "[" ~ lenString ~ "]"))();
}

override bool opEquals(Object o)
Expand Down Expand Up @@ -1202,7 +1204,7 @@ class TypeInfo_Vector : TypeInfo

class TypeInfo_Function : TypeInfo
{
override string toString() const @trusted
override string toString() const pure @trusted
{
import core.demangle : demangleType;

Expand Down Expand Up @@ -1279,7 +1281,7 @@ class TypeInfo_Function : TypeInfo

class TypeInfo_Delegate : TypeInfo
{
override string toString() const @trusted
override string toString() const pure @trusted
{
import core.demangle : demangleType;

Expand Down Expand Up @@ -1399,7 +1401,7 @@ private extern (C) int _d_isbaseof(scope TypeInfo_Class child,
*/
class TypeInfo_Class : TypeInfo
{
override string toString() const { return info.name; }
override string toString() const pure { return info.name; }

override bool opEquals(Object o)
{
Expand Down Expand Up @@ -1584,7 +1586,7 @@ alias ClassInfo = TypeInfo_Class;

class TypeInfo_Interface : TypeInfo
{
override string toString() const { return info.name; }
override string toString() const pure { return info.name; }

override bool opEquals(Object o)
{
Expand Down Expand Up @@ -1703,13 +1705,17 @@ class TypeInfo_Struct : TypeInfo
{
override string toString() const { return name; }

override size_t toHash() const
{
return hashOf(this.mangledName);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeInfo.opCmp uses toString() too; I'm not convinced overriding it here to check for a TypeInfo_Struct rhs and comparing the mangledName instead is worth it.

override bool opEquals(Object o)
{
if (this is o)
return true;
auto s = cast(const TypeInfo_Struct)o;
return s && this.name == s.name &&
this.initializer().length == s.initializer().length;
return s && this.mangledName == s.mangledName;
}

override size_t getHash(scope const void* p) @trusted pure nothrow const
Expand Down Expand Up @@ -1794,7 +1800,29 @@ class TypeInfo_Struct : TypeInfo
(*xpostblit)(p);
}

string name;
string mangledName;

final @property string name() nothrow const @trusted
{
import core.demangle : demangleType;

if (mangledName is null) // e.g., opaque structs
return null;

const key = cast(const void*) this; // faster lookup than TypeInfo_Struct, at the cost of potential duplicates per binary
static string[typeof(key)] demangledNamesCache; // per thread

// not nothrow:
//return demangledNamesCache.require(key, cast(string) demangleType(mangledName));

if (auto pDemangled = key in demangledNamesCache)
return *pDemangled;

const demangled = cast(string) demangleType(mangledName);
demangledNamesCache[key] = demangled;
return demangled;
}

void[] m_init; // initializer; m_init.ptr == null if 0 initialize

@safe pure nothrow
Expand Down
4 changes: 2 additions & 2 deletions src/rt/aaA.d
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ TypeInfo_Struct fakeEntryTI(ref Impl aa, const TypeInfo keyti, const TypeInfo va
extra[0] = cast() kti;
extra[1] = cast() vti;

static immutable tiName = __MODULE__ ~ ".Entry!(...)";
ti.name = tiName;
static immutable tiMangledName = "S2rt3aaA__T5EntryZ";
ti.mangledName = tiMangledName;

ti.m_RTInfo = rtisize > 0 ? rtinfoEntry(aa, keyinfo, valinfo, cast(size_t*)(extra + 2), rtisize) : rtinfo;
ti.m_flags = ti.m_RTInfo is rtinfoNoPointers ? cast(TypeInfo_Struct.StructFlags)0 : TypeInfo_Struct.StructFlags.hasPointers;
Expand Down
10 changes: 5 additions & 5 deletions src/rt/util/typeinfo.d
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,12 @@ class TypeInfo_v : TypeInfoGeneric!ubyte
{
return 1;
}
}

unittest
{
assert(typeid(void).toString == "void");
assert(typeid(void).flags == 1);
}
unittest
{
assert(typeid(void).toString == "void");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeInfo.toString() isn't pure anymore.

assert(typeid(void).flags == 1);
}

// All integrals.
Expand Down