Skip to content

Decrease compile time#77

Merged
s-ludwig merged 1 commit intos-ludwig:masterfrom
burner:compile_time_improvments
Aug 4, 2025
Merged

Decrease compile time#77
s-ludwig merged 1 commit intos-ludwig:masterfrom
burner:compile_time_improvments

Conversation

@burner
Copy link
Contributor

@burner burner commented Jul 31, 2025

std.format and std.numeric.gcd shows up quite heavily during tracing compile times.
This PR removes two expansive format calls and introduces a super simple gcd algorithm.

Below are two traces building vibe.d with and without the changes. With the change compilation is about 7% faster on my machine.
trace_baseline.json
trace.json

I'm a big std.format fan, but its compile time is just bad.

@burner burner force-pushed the compile_time_improvments branch from 2d6aeb3 to 0ee0acb Compare July 31, 2025 20:51
Copy link
Owner

@s-ludwig s-ludwig left a comment

Choose a reason for hiding this comment

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

Funny coincidence that I was looking at template instantiations yesterday and also hit taggedalgebraic (mostly within visit, though). However, the improvements I got were within measuring tolerance, so I didn't follow through with it.

{
static if (isUnitType!T) enum isHashable = true;
else static if (__traits(compiles, (ref const(T) val) @safe nothrow => hashOf(val)))
else static if (__traits(compiles, canHashOf(T.init)))
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't T.init generally not an lvalue, so that this will fail at the call site for canHashOf?

@burner burner force-pushed the compile_time_improvments branch from 0ee0acb to 12a90aa Compare August 1, 2025 13:36
@burner
Copy link
Contributor Author

burner commented Aug 1, 2025

Funny coincidence that I was looking at template instantiations yesterday and also hit taggedalgebraic (mostly within visit, though). However, the improvements I got were within measuring tolerance, so I didn't follow through with it.

My terrible rule of thumb is that when compile times are bad is always std.format. Especially, CTFE with AliasSeq resulting in mixins. std.format is great it just compiles so slow.

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 1, 2025

Yeah, format and related things were very high up on the list, but in the overall picture, there were so many sources for template instantiations that I was rather thinking about looking at possible improvements in std.format directly. Since this had such a big impact, I should probably rethink that and look for similarly impactful format uses.

By the way, the __traits(compiles) change, what does that aimed to improve? It's a lot of extra code, but I don't see how that would improve compile times. Using T val; instead of just a ref parameter with no actual value passed will also make types with disabled default construction (@disable this();) fail the test. It would have to be something like T* pval; canHashOf(*pval); instead.

@burner
Copy link
Contributor Author

burner commented Aug 1, 2025

As far as I can see its not more code or templates per se, but its closeness to old style C.
I'm not a compiler dev, I can't give the correct reason, sorry.

My approach is pretty simple, add

  1. "dflags-ldc": ["-d", "-ftime-trace", "-ftime-trace-file=$PACKAGE_DIR/trace.json", "-wi"] to dub.json
  2. compile
  3. inspect with chrome://tracing
  4. look if its faster
  5. look at the code of the low and remove format and nested templates
  6. goto 2

I agree std.format should compile faster, but I haven't gotten Walter yet to take a look.

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 3, 2025

As far as I can see its not more code or templates per se, but its closeness to old style C.
I'm not a compiler dev, I can't give the correct reason, sorry.

But you are certain that the specific change at line 784/800 does improve compile time, or is it just an independent relic?

@burner
Copy link
Contributor Author

burner commented Aug 3, 2025

But you are certain that the specific change at line 784/800 does improve compile time, or is it just an independent relic?

I'm not quite sure what you mean independent relic, but stepping through the compile trace this is a thing where the compile time is less of an overall smaller compile time in general

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 3, 2025

I mean the main change is obviously avoiding the format calls for generating the constructor source code, as well as replacing gcd with a custom implementation. The change in isHashable looks to be completely independent of this, no?

@burner
Copy link
Contributor Author

burner commented Aug 3, 2025

The change in isHashable looks to be completely independent of this, no?

it is. its just avoids the creation of the delegate based on the template.

Just say the word and I'll revert that change, its not the biggest win in comparison to the other changes.

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 3, 2025

Since it changes semantics and in its current form creates two template function instantiations instead of one, I'd say we should just leave it out, unless there are in fact significant gains.

@burner burner force-pushed the compile_time_improvments branch from 12a90aa to 276e8f8 Compare August 4, 2025 12:14
@burner
Copy link
Contributor Author

burner commented Aug 4, 2025

should just leave it out

done

std.format and std.numeric.gcd shows up quite heavily during
tracing compile times.
This PR removes two expansive format calls and introduces a super simple
gcd algorithm.
Copy link
Owner

@s-ludwig s-ludwig left a comment

Choose a reason for hiding this comment

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

Okay, thanks! Merging.

@s-ludwig s-ludwig merged commit 9f0b79a into s-ludwig:master Aug 4, 2025
12 checks passed
@burner
Copy link
Contributor Author

burner commented Aug 4, 2025

@s-ludwig thank you for taking the time

@burner
Copy link
Contributor Author

burner commented Aug 6, 2025

@s-ludwig any change I could get a v1.0.1 release?

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 6, 2025

@burner Sure, I'm just waiting for #78 to get merged, too, and will then create a tag.

@burner
Copy link
Contributor Author

burner commented Aug 7, 2025

@s-ludwig thank you

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 7, 2025

Okay, code.dlang.org is updated with 1.0.1 now.

@burner
Copy link
Contributor Author

burner commented Aug 7, 2025

awesome, thank you my best project saves 0.2 seconds on a 2.5 seconds build and the biggest thing is codegen in ldc (about 1.2s) the frontend is mostly dpql denizzzka/dpq2#214

@s-ludwig
Copy link
Owner

s-ludwig commented Aug 9, 2025

@burner There appears to be a pretty big compile-time improvement/regression fix related to format instantiations coming with the next compiler release: vibe-d/vibe-serialization#23 (comment)

@burner
Copy link
Contributor Author

burner commented Aug 10, 2025

@burner There appears to be a pretty big compile-time improvement/regression fix related to format instantiations coming with the next compiler release: vibe-d/vibe-serialization#23 (comment)

that would be awesome, I'll keep an eye open thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants