Switch to faster version of staticMap

[nordlow]

Now that

• https://issues.dlang.org/show_bug.cgi?id=22421 has been fixed via Fix Issue 22421 - static foreach introduces semantic difference between indexing and iteration variable dmd#13252 and
• quadratic behavior of alias assign has been removed via Optimize AliasAssign tuple building dmd#14332.

When compiling https://github.com/nordlow/phobos-next/blob/master/snippets/benchmark_staticMap.d,
the first implementation is the fastest and least memory consuming.

Implementations:

1. 0.08s 90 Mb
2. 0.12s 109 Mb
3. 0.12s 109 Mb

FYI, @andralex @atilaneves.

[dlang-bot]

Thanks for your pull request and interest in making D better, @nordlow! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your 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 locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8535"

[RazvanN7]

I wonder if we should delete the other versions of staticMap.

[nordlow]

I wonder if we should delete the other versions of staticMap.

Yes, I think so. Ok with you, @andralex?

[atilaneves]

I wonder if we should delete the other versions of staticMap.

Yes, there's no good reason to keep them around.

[andralex]

Fantastic work, @BorisCarvajal and thanks @nordlow for following up! So satisfying to see the simplest, natural implementation also be fastest!

[RazvanN7]

@nordlow Please delete the other versions and let's get this in!

[nordlow]

Btw, regarding https://github.com/nordlow/phobos-next/blob/master/snippets/benchmark_staticMap.d

regarding

0.08s 90 Mb
0.12s 109 Mb
0.12s 109 Mb

I wonder why the second implementation is a slow as the third. Is there a reason why dmd cannot optimize it as good as the first @BorisCarvajal?

[nordlow]

Btw, the staticMap reversed at https://github.com/nordlow/phobos-next/blob/master/snippets/benchmark_staticMap_reversed.d gives the same speedup for the first case. Nice.

[nordlow]

@nordlow Please delete the other versions and let's get this in!

Resolved via https://github.com/dlang/phobos/compare/7298da2cf9efbfd05d9f74d2991fa8dd26c66fea..76c4a43c929fb5fb4cd8296526d46e0f465a8ebb.

Ready for merge!

[BorisCarvajal]

I wonder why the second implementation is a slow as the third. Is there a reason why dmd cannot optimize it as good as the first @BorisCarvajal?

Well, there are no optimizations at all, dmd just unrolls foreach body.

I suppose the difference is because:
First case just uses an alias to the element of the tuple (or the element as var initializer if TupExp).
Second case creates an array of index values first, then there is an additional tuple indexing that need to be resolved in each unrolled iteration.
I have a nearly finished patch for static foreach that removes the unnecessary array, among other things. But the other one involves analyzing the foreach body to check if it's possible a rewrite to the first case. Seems impractical tbh.