-
-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize impcnvtab for newCTFE #9112
Conversation
Thanks for your pull request, @UplinkCoder! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#9112" |
37582f7
to
bdf5897
Compare
Within the inner functions the 23232 byte large impcnvtab is used, because of how delegates and closures are implemented in newCTFE the full copy has to be done everytime the table is touched. Therefore I opted to pass a pointer explicitly, which improves compiletimes for newCTFE by a factor of 500
bdf5897
to
366ab34
Compare
Nice! What are the numbers for old CTFE? |
Haven't checked, probably little to nothing. I don't know how they implement delegates. |
This should most likely be the other way around - optimizing newCTFE instead, to prevent this ugliness and slowness (factor of 500?!) for similar code. |
currently I cannot devote resources to optimizing border cases like giant structs being passed via closure variables. It's quite hard to fix actually since newCTFE has no notion of a stack. |
I fully understand, but I don't see a reason for this to be in upstream DMD, it just uglifies the code unless someone can show it improves the current interpreter performance too. |
border cases? Hello regex? |
This PR doesn't improve the code readability, and I don't think the change carries its weight. |
Within the inner functions the 23232 byte large impcnvtab is used,
because of how delegates and closures are implemented in newCTFE
the full copy has to be done everytime the table is touched.
Therefore I opted to pass a pointer explicitly,
which improves compiletimes for newCTFE by a factor of 500