Skip to content

perf: optimize OpamString.split - #6210

Merged
kit-ty-kate merged 2 commits into
ocaml:masterfrom
kit-ty-kate:speedup-string-split
Dec 17, 2024
Merged

perf: optimize OpamString.split#6210
kit-ty-kate merged 2 commits into
ocaml:masterfrom
kit-ty-kate:speedup-string-split

Conversation

@kit-ty-kate

@kit-ty-kate kit-ty-kate commented Sep 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

this cuts the time spent on parsing pacman -Si significantly down

Commit by @c-cube split from #5757
Queued on #6212

@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Sep 25, 2024
@kit-ty-kate

kit-ty-kate commented Sep 25, 2024

Copy link
Copy Markdown
Member Author

After trying a couple of things, it looks like opam show dune shows the significant performance improvement (25% on my local Fedora), however Debian (the system used for benchmark) shows less improvement (6 or 7%). I'll add a benchmark nonetheless and see if it can be reproduced in a better controlled environment

@kit-ty-kate kit-ty-kate mentioned this pull request Sep 25, 2024
Merged
@kit-ty-kate kit-ty-kate added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Sep 25, 2024
@rjbou
rjbou self-requested a review September 30, 2024 12:27
@kit-ty-kate
kit-ty-kate marked this pull request as draft September 30, 2024 12:28
@kit-ty-kate

kit-ty-kate commented Dec 5, 2024

Copy link
Copy Markdown
Member Author

ok i should've just started with benchmarking just the function in isolation. It now shows a 45x speedup

@kit-ty-kate
kit-ty-kate marked this pull request as ready for review December 5, 2024 06:27
rjbou
rjbou approved these changes Dec 16, 2024
View reviewed changes

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! 🚀

@kit-ty-kate kit-ty-kate removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Dec 17, 2024
@c-cube @kit-ty-kate
perf: optimize OpamString.split
30827b0 
this cuts the time spent on parsing `pacman -Si` significantly down

Co-authored-by: Kate <kit-ty-kate@outlook.com>
@kit-ty-kate
kit-ty-kate merged commit 6dfc1ec into ocaml:master Dec 17, 2024
@kit-ty-kate
kit-ty-kate deleted the speedup-string-split branch December 17, 2024 17:07
@rjbou

rjbou commented Dec 17, 2024

Copy link
Copy Markdown
Collaborator

60s to 1.2s, from bench

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

Reviewers

@rjbou rjbou rjbou approved these changes

Assignees

No one assigned

Labels

AREA: PERFORMANCE

Projects

None yet

Milestone

2.4.0~alpha1

Development

Successfully merging this pull request may close these issues.

3 participants

@kit-ty-kate @rjbou @c-cube