Skip to content

Reimplement Source::indexOf(ByteString) without Source::peek calls #242

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 30, 2023
Merged

Reimplement Source::indexOf(ByteString) without Source::peek calls #242

merged 4 commits into from
Nov 30, 2023

Conversation

fzhinkin
Copy link
Collaborator

As in #240, reimplemented algorithm without the use of the PeekSource. Also added a specialized implementation for Buffer.

@fzhinkin fzhinkin requested a review from shanshin November 28, 2023 16:15
@fzhinkin
Copy link
Collaborator Author

Here are benchmarking results (I executed benchmarks in throughput mode, so higher the score, better the performance):

baseline (devel):

Benchmark                    (params)   Mode  Cnt       Score      Error  Units
IndexOfByteString.benchmark    1024:2  thrpt    5  171907.309 ± 6796.564  ops/s
IndexOfByteString.benchmark    8192:2  thrpt    5   19421.300 ±  401.089  ops/s
IndexOfByteString.benchmark   10000:2  thrpt    5   16095.808 ±  163.348  ops/s
IndexOfByteString.benchmark   10000:8  thrpt    5    8474.794 ±   24.459  ops/s

new version (this branch):

Benchmark                    (params)   Mode  Cnt       Score       Error  Units
IndexOfByteString.benchmark    1024:2  thrpt    5  534757.836 ± 11554.425  ops/s
IndexOfByteString.benchmark    8192:2  thrpt    5   67568.970 ±   244.761  ops/s
IndexOfByteString.benchmark   10000:2  thrpt    5   51233.638 ±   365.530  ops/s
IndexOfByteString.benchmark   10000:8  thrpt    5   30259.195 ±   421.080  ops/s

@fzhinkin fzhinkin marked this pull request as ready for review November 28, 2023 16:16
@fzhinkin fzhinkin self-assigned this Nov 28, 2023
shanshin
shanshin reviewed Nov 29, 2023
View reviewed changes
core/common/src/ByteStrings.kt
val peek = peek()
if (!request(startIndex)) {
return -1L
while (request(offset + byteString.size)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just write an idea for the future: if a request appears, it may be worth adding a modification of the function to which fetchSize can be passed. For example, it can help if byteString.size == 2, so that there is no frequent reading of two bytes (which will most likely be slower than reading a large chunk)

@fzhinkin fzhinkin merged commit 0d2aa85 into develop Nov 30, 2023
fzhinkin added a commit that referenced this pull request Jan 4, 2024
@fzhinkin
Reimplement Source::indexOf(ByteString) without Source::peek calls (#242
75e2195 
)

Reimplement indexOf(ByteString) without the use of peek-source. Along with direct access into buffer's data (instead of using `Buffer::get`) it gave a significant performance boost.
@fzhinkin fzhinkin deleted the indexof-bytestring-wo-peek branch August 27, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shanshin shanshin shanshin approved these changes

Assignees

@fzhinkin fzhinkin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fzhinkin @shanshin