Skip to content

Conversation

@SergeiPavlov
Copy link
Collaborator

Because (a ^ b) == 0 if and only if a == b

@botinko
Copy link

botinko commented Nov 15, 2025

Why it was like that before?

@botinko
Copy link

botinko commented Nov 15, 2025

I found it wasdone by AU 971370b
I remember he did a lot of benchmarking for this case. Maybe xor approach is faster?

@SergeiPavlov
Copy link
Collaborator Author

SergeiPavlov commented Nov 15, 2025

I remember he did a lot of benchmarking for this case. Maybe xor approach is faster?

I think he just copy/pasted this pattern from some other project.
See:
5119c1f#diff-4debfe6bcc478710248f83f0466c51fe370e88f121efc6f3875a40095f08124aR943
&
5119c1f#diff-4debfe6bcc478710248f83f0466c51fe370e88f121efc6f3875a40095f08124aR949

He used both: == & ^ in the same function

I don't belive the compiler can generate more optimal code than single CMP instruction to compare two ints

Copy link

@snaumenko-st snaumenko-st left a comment

Choose a reason for hiding this comment

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

I think that we can also change

ReferenceEquals(Module1, Module2)

with

Module1.ModuleHandle == Module2.ModuleHandle

@SergeiPavlov
Copy link
Collaborator Author

I think that we can also change

ReferenceEquals() looks better. There is no pointer dereference while Module1.ModuleHandle == Module2.ModuleHandle reads two values from RAM to compare

@SergeiPavlov SergeiPavlov merged commit 4618465 into master-servicetitan Nov 17, 2025
38 checks passed
@SergeiPavlov SergeiPavlov deleted the MetadataToken branch November 17, 2025 21:12
@snaumenko-st
Copy link

snaumenko-st commented Nov 17, 2025

I think that we can also change

ReferenceEquals() looks better. There is no pointer dereference while Module1.ModuleHandle == Module2.ModuleHandle reads two values from RAM to compare

I did some benchmarks earlier and ModuleHandle comparison was 10% faster than reference. Probably reference comparison involves more checks than in case of structs, or it may be something else... Probably it's worth to repeat the tests again.

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