Skip to content
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

Replace J2N.Numerics.BitOperation.TripleShift with >>> in C# 11 #1006

Closed
1 task done
paulirwin opened this issue Nov 2, 2024 · 2 comments · Fixed by #1007
Closed
1 task done

Replace J2N.Numerics.BitOperation.TripleShift with >>> in C# 11 #1006

paulirwin opened this issue Nov 2, 2024 · 2 comments · Fixed by #1007
Assignees
Labels
is:task A chore to be done

Comments

@paulirwin
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Task description

There are various places throughout the codebase where we are using the TripleShift extension method from J2N to perform an unsigned right shift, since older versions of C# did not have this operator. C# 11 added the >>> operator like Java has to do this, which gets translated directly to the shr.un opcode, so should be more efficient than the several opcodes involved in the various TripleShift overloads, even if it is aggressively inlined. In addition, it will help make direct porting from Java easier.

(Note: the aggressive inlining doesn't seem to be working in our published NuGet packages, as spot checking one example in BaseCharFilter still had a call operation to TripleShift, which is certainly going to be worse for performance than a single opcode.)

I have confirmed this works on net462, our oldest target, with LangVersion set to 11, for all overloads that TripleShift supports.

Need to confirm that setting our LangVersion to 11 does not cause regressions elsewhere.

@NightOwl888
Copy link
Contributor

I have added this task to the other projects we own where it applies:

NightOwl888/J2N#115
NightOwl888/ICU4N#92
NightOwl888/Morfologik.Stemming#1
NightOwl888/RandomizedTesting#5

@paulirwin
Copy link
Contributor Author

Cool, thanks. I can take those

@paulirwin paulirwin added this to the 4.8.0-beta00018 milestone Nov 3, 2024
paulirwin added a commit that referenced this issue Nov 3, 2024
…ft operator (#1007)

* SWEEP: Replace J2N's TripleShift call with C# 11's unsigned right shift operator, #1006

* Fix failing unit tests on <= .NET 6

* Remove unnecessary parentheses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:task A chore to be done
Projects
None yet
2 participants