Skip to content

fix #21976: [REG 2.112.0] Array comparison performance regression#22522

Merged
thewilsonator merged 1 commit intodlang:stablefrom
rainers:issue21976
Feb 6, 2026
Merged

fix #21976: [REG 2.112.0] Array comparison performance regression#22522
thewilsonator merged 1 commit intodlang:stablefrom
rainers:issue21976

Conversation

@rainers
Copy link
Member

@rainers rainers commented Feb 6, 2026

restore going through __equals lowering for all arrays but strings.

No idea how to test the performance regression that doesn't manifest everywhere.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @rainers!

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.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

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 "stable + dmd#22522"

@rainers
Copy link
Member Author

rainers commented Feb 6, 2026

This restores the check from
https://github.com/dlang/dmd/pull/21513/changes#diff-a556a8e6917dd4042f541bdb19673f96940149ec3d416b0156af4d0e4cc5e4bdL13483

but I have no idea why strings have to be excluded (though repe cmpsb might be ok there). Otherwise I get

..\..\phobos\std\stdio.d(5262): Error: can't infer return type in function `makeGlobal`
                    assert(_iob == stdin || _iob == stdout || _iob == stderr);
                                ^
..\..\phobos\std\stdio.d(5262): Error: can't infer return type in function `makeGlobal`
                    assert(_iob == stdin || _iob == stdout || _iob == stderr);
                                                 ^
..\..\phobos\std\stdio.d(5262): Error: can't infer return type in function `makeGlobal`
                    assert(_iob == stdin || _iob == stdout || _iob == stderr);
                                                                   ^

when compiling with -inline.

restore going through __equals lowering for all arrays but strings
@rainers
Copy link
Member Author

rainers commented Feb 6, 2026

No idea how to test the performance regression that doesn't manifest everywhere.

Ah, there is already a test that checks whether a lowering is used or not. Added __equals there.

@kinke
Copy link
Contributor

kinke commented Feb 6, 2026

#21513 removed the memcmp-optimization from __equals, so this should make things even slower AFAICT (element-wise comparisons in a loop). - I think this should be handled at the DMD backend level, using the library-memcmp for statically unknown or non-small sizes.

@kinke
Copy link
Contributor

kinke commented Feb 6, 2026

That said, I think #21513 was actually the wrong way around, moving this use-memcmp logic from druntime back to the compiler (AFAICT, just to have a direct memcmp/rep call for trivially comparable arrays, instead of a __equals() performing the call). E.g., the logic was previously nicely tested via unittests, whereas the compiler logic has no unittests at all (as it's much harder).

@rainers
Copy link
Member Author

rainers commented Feb 6, 2026

#21513 removed the memcmp-optimization from __equals, so this should make things even slower

I saw similar results as with dmd 2.111, so I assumed that there was some memcmp in the __equals implementation. Maybe that doesn't hold when comparing to the glibc-memcmp.

I think this should be handled at the DMD backend level

I agree that would be better. I first looked at whether that would be feasible, just to notice that quite some more "backend-opcodes" are implemented doing rep-byte operations. I don't really know the backend and don't want to dive that deep.

That would be for someone else to implement...

I think #21513 was actually the wrong way around

I also wondered why all these backend and architecture specific details are handled in the frontend, determining whether to invoke __equals or not.

@thewilsonator thewilsonator merged commit e5ca2f1 into dlang:stable Feb 6, 2026
78 checks passed
@kinke
Copy link
Contributor

kinke commented Feb 6, 2026

I saw similar results as with dmd 2.111, so I assumed that there was some memcmp in the __equals implementation. Maybe that doesn't hold when comparing to the glibc-memcmp.

I'm sure a byte[] comparison is now still much slower than with v2.111's memcmp (in __equals()), especially on Linux with a glibc memcmp. And this now prematurely merged means the other compilers are affected too, using the element-wise comparison in __equals() (which LLVM/GDC might be able to optimize to a memcmp again, but only with much more effort...).

@rainers
Copy link
Member Author

rainers commented Feb 6, 2026

Sorry, I measured a slightly modified version, the original test has become a lot worse. Let's revert...

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.

4 participants