Add betterC probing for performance and for applications without drun… #2753
Conversation
|
✅ PR OK, no changes in deprecations or warnings Total deprecations: 11 Total warnings: 0 Build statistics: statistics (-before, +after)
-executable size=5371736 bin/dub
-rough build time=62s
+executable size=5375832 bin/dub
+rough build time=63sFull build output |
|
Duplicate of: #2699 |
|
My solution involves minor changes and reverts to old behaviour, easier to approve. |
source/dub/compilers/compiler.d
Outdated
| @@ -183,18 +183,32 @@ interface Compiler { | |||
| import dub.compilers.utils : generatePlatformProbeFile, readPlatformJsonProbe; | |||
| import std.string : format, strip; | |||
|
|
|||
| auto fil = generatePlatformProbeFile(); | |||
| NativePath fil = generatePlatformProbeFile!true(); | |||
| auto result_betterC = execute(compiler_binary ~ args ~ "-betterC" ~ fil.toNativeString()); | |||
There was a problem hiding this comment.
-betterC is called something else with GDC ,-f-no-druntime or something like that.
There was a problem hiding this comment.
Seems to be -fno-druntime
source/dub/compilers/utils.d
Outdated
| enum moduleInfo = betterC ? q{ | ||
| module object; | ||
| alias string = const(char)[]; | ||
| alias size_t = uint; | ||
| template _d_arrayappendcTXImpl(Tarr : T[], T){ | ||
| ref Tarr _d_arrayappendcTX(return ref scope Tarr px, size_t n) @trusted pure nothrow{return px;} | ||
| } | ||
| const(char)[][] _d_arrayappendcTX(return scope ref const(char)[][] px, size_t n){return px;} | ||
| Tarr _d_arrayappendT(Tarr : T[], T)(return ref scope Tarr x, scope Tarr y){return x;} | ||
| } : q{ | ||
| module dub_platform_probe; | ||
| }; |
There was a problem hiding this comment.
I'm a bit concerned about this part, as it seems to be specific to the current state of BetterC. Future improvements might make this out of date. Am I right that the problem arises because the platform probe has a dependency on the runtime ? Couldn't we achieve something similar with a sufficiently sized static array ?
There was a problem hiding this comment.
It's the string[] appender running as CTFE that calls the runtime as a noop, if there's any improvements we would be removing some definitions and that's pretty much it. Either way if anything breaks it'll show a warning and fallback to using the runtime (through the bool retry in the catch)
There was a problem hiding this comment.
The CTFE doesn't support using static storage either, you have to use the GC to create an allocation and then you can reference it in scope
There was a problem hiding this comment.
I managed to remove the declarations by using only pragma(msg, ) and formatting the string at the end
etcimon
force-pushed
the
probe_with_betterc
branch
4 times, most recently
from
7d35ef2 to
a342d7e
Compare
etcimon
force-pushed
the
probe_with_betterc
branch
from
a342d7e to
2250725
Compare
| @@ -16,6 +16,7 @@ __dummy.html | |||
| /bin/dub-test-library | |||
| /bin/libdub.a | |||
| /bin/dub-* | |||
| /bin/dub.* | |||
There was a problem hiding this comment.
It's a minor annoyance when using vscode source control on windows, I was thinking it didn't require mention, nor its own pull request.
There was a problem hiding this comment.
What artifacts does vscode generate ? We can keep it in this PR, I just could not tell where it's coming from.
There was a problem hiding this comment.
This is for the dub.exe and dub.pdb files that are generated when building under windows.
|
Just to be sure I follow: Do we need the |
No, it doesn't. You will still be linking against druntime, and that has cost. |
|
The If I benchmark only the first run on windows arch x86_64, it goes from 52ms to 24ms |
|
On dmd removing the |
Why do we need to link ? |
|
It wouldn't link, it doesn't write an object file, compile flag |
So why is |
|
Originally it's because this project https://github.com/etcimon/libwasm implements its own druntime and for it you must remove the includes in ldc2.conf, but probing fails on that configuration if it depends on druntime. |
I assume that probing fails because it tries to import druntime, rather than link it ? |
|
It complains that there is no |
|
@kinke : Any chance you could have a look at this ? I think the use case is good, but I would like to make sure it keeps on working with newer versions of DMD/LDC with minimal maintenance. |
It works without |
|
Are we waiting on @kinke for a follow-up? |
|
AFAICT after a quick glance, the main change here is that the generated probe module becomes the special While such a minimalistic, fully self-contained D module is surely fast to analyze with |
The post-processing using regex, matchAll and replace makes it impossible to evaluate the string with ctfe, but I doubt this would make it slower. |
etcimon
force-pushed
the
probe_with_betterc
branch
from
169a3cc to
fece411
Compare
etcimon
force-pushed
the
probe_with_betterc
branch
from
1447625 to
01a8156
Compare
|
nvm I got it working in ctfe |
source/dub/compilers/compiler.d
Outdated
| BuildPlatform build_platform; | ||
|
|
||
| auto build_platform = readPlatformJsonProbe(result.output); | ||
| build_platform = readPlatformSDLProbe(result.output); |
source/dub/compilers/compiler.d
Outdated
| if (ver.empty) { | ||
| logWarn(`Could not probe the compiler version for "%s". ` ~ | ||
| `Toolchain requirements might be ineffective`, build_platform.compiler); | ||
| `Toolchain requirements might be ineffective`, build_platform.compiler); |
source/dub/compilers/utils.d
Outdated
| @@ -268,78 +268,103 @@ private enum probeEndMark = "__dub_probe_end__"; | |||
| */ | |||
| NativePath generatePlatformProbeFile() | |||
| { | |||
|
|
|||
source/dub/compilers/utils.d
Outdated
| module object; | ||
| alias string = const(char)[]; | ||
| }; | ||
|
|
||
| // try to not use phobos in the probe to avoid long import times |
There was a problem hiding this comment.
I would also modify the comment so the next person knows the platform probe should not rely on the runtime to make your use case possible.
|
Should I squash it? |
This improves performance and also allows me to compile a custom druntime with my application. (post-switches = [] in the ldc2.conf)