Skip to content

improve ByteString lastIndexOf#2153

Closed
pjfanning wants to merge 1 commit intoapache:mainfrom
pjfanning:lastIndexOf-improve
Closed

improve ByteString lastIndexOf#2153
pjfanning wants to merge 1 commit intoapache:mainfrom
pjfanning:lastIndexOf-improve

Conversation

@pjfanning
Copy link
Copy Markdown
Member

@pjfanning pjfanning commented Sep 5, 2025 •
edited
Loading

  • see Optimise ByteString lastIndexOf #2150
  • WIP
  • doesn't yet use SWAR but having the new methods that take Byte inputs instead of the [B >: Byte] versions is the biggest win
  • needs a jmh benchmark

@pjfanning pjfanning changed the title Last index of improve improve ByteString lastIndexOf Sep 5, 2025
@pjfanning pjfanning marked this pull request as draft September 5, 2025 21:18
@He-Pin He-Pin added this to the 2.0.0-M1 milestone Sep 6, 2025
@pjfanning pjfanning modified the milestones: 2.0.0-M1, 2.0.0-M2 Oct 8, 2025
@pjfanning
ByteStrings impl
b5a4ec5 
add optimised lastIndexOf

Update ByteString.scala

Update ByteString.scala

Update ByteString.scala
@pjfanning pjfanning force-pushed the lastIndexOf-improve branch from fab09a7 to b5a4ec5 Compare October 8, 2025 17:32
@pjfanning pjfanning modified the milestones: 2.0.0-M2, 2.0.0-M3 Apr 4, 2026
He-Pin
He-Pin reviewed Apr 6, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

Good performance improvement for ByteString.lastIndexOf. A few observations:

  1. Specialized Byte overload: Adding the Byte-typed overload (avoiding the [B >: Byte] generic version) is indeed the biggest win since it avoids boxing/unboxing.

  2. SWAR optimization: The PR mentions it doesn't yet use SWAR (SIMD Within A Register). That would be a nice follow-up for even better performance on longer ByteStrings.

  3. Missing JMH benchmark: As noted in the PR body, a JMH benchmark is needed to quantify the improvement. This should be added before merge.

  4. Staleness: Open since September 2025 with no reviews yet. The implementation looks solid -- the main gap is the benchmark. @pjfanning would you like to finish this up?

@pjfanning pjfanning closed this Apr 8, 2026
@pjfanning
Copy link
Copy Markdown
Member Author

pjfanning commented Apr 8, 2026

going with #2838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@He-Pin He-Pin He-Pin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.0.0-M3

Development

Successfully merging this pull request may close these issues.

2 participants

@pjfanning @He-Pin