Skip to content

Optimize maxAlignment compile time #6799

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.

Sign up for GitHub

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

Merged
dlang-bot merged 1 commit into from
Dec 29, 2018
Merged

Optimize maxAlignment compile time #6799

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions std/traits.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4751,15 +4751,17 @@ template TemplateArgsOf(T : Base!Args, alias Base, Args...)
private template maxAlignment(U...)
if (isTypeTuple!U)
{
import std.meta : staticMap;
static if (U.length == 0)
static assert(0);
else static if (U.length == 1)
enum maxAlignment = U[0].alignof;
else static if (U.length == 2)
enum maxAlignment = U[0].alignof > U[1].alignof ? U[0].alignof : U[1].alignof;
else
{
import std.algorithm.comparison : max;
enum maxAlignment = max(staticMap!(.maxAlignment, U));
enum a = maxAlignment!(U[0 .. ($+1)/2]);
enum b = maxAlignment!(U[($+1)/2 .. $]);
enum maxAlignment = a > b ? a : b;
Copy link
Contributor

@thewilsonator thewilsonator Dec 10, 2018

Choose a reason for hiding this comment

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

It may well be worth unrolling the template recursion a bit too to limit the number of template instantiations.

Copy link
Contributor Author

@TurkeyMan TurkeyMan Dec 10, 2018

Choose a reason for hiding this comment

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

See how I did a logN expansion rather than a linear recursive expansion? I think that probably does the job, no?

Copy link
Contributor Author

@TurkeyMan TurkeyMan Dec 10, 2018

Choose a reason for hiding this comment

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

Also, compared to instantiating max, this is practically nothing.

Copy link
Contributor Author

@TurkeyMan TurkeyMan Dec 10, 2018

Choose a reason for hiding this comment

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

...but I was also pondering whether I should use CTFE rather than a template recursion.
I don't know enough about the cost/benefit tradeoff on that front. At what point does CTFE become profitable?

Copy link
Contributor

@thewilsonator thewilsonator Dec 10, 2018

Choose a reason for hiding this comment

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

Probably always, @UplinkCoder ?

Copy link
Contributor Author

@TurkeyMan TurkeyMan Dec 10, 2018

Choose a reason for hiding this comment

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

Also, truth is that U.length almost always == 2

Copy link
Contributor

@thewilsonator thewilsonator Dec 10, 2018

Choose a reason for hiding this comment

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

In which case just do a static if (U.length == 2) branch ;)

Copy link
Contributor Author

@TurkeyMan TurkeyMan Dec 10, 2018

Choose a reason for hiding this comment

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

did

Copy link
Member

@UplinkCoder UplinkCoder Dec 19, 2018

Choose a reason for hiding this comment

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

CTFE is better in almost all cases, except maybe for a N ∈ {0 ... 5} :)

}
}

Expand Down