Skip to content

perf(key): optimize Matches#261

Open
knz wants to merge 2 commits intocharmbracelet:masterfrom
knz:20221002-key
Open

perf(key): optimize Matches#261
knz wants to merge 2 commits intocharmbracelet:masterfrom
knz:20221002-key

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Oct 2, 2022

Fixes #262.

name                old time/op    new time/op    delta
Matches/success-32    66.9ns ± 1%    15.7ns ± 0%   -76.47%  (p=0.016 n=5+4)
Matches/fail-32       27.9ns ± 0%    12.4ns ± 2%   -55.70%  (p=0.016 n=4+5)

name                old alloc/op   new alloc/op   delta
Matches/success-32     5.00B ± 0%     0.00B       -100.00%  (p=0.008 n=5+5)
Matches/fail-32        0.00B          0.00B           ~     (all equal)

name                old allocs/op  new allocs/op  delta
Matches/success-32      1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Matches/fail-32         0.00           0.00           ~     (all equal)

cc @meowgorithm @muesli

knz added 2 commits October 2, 2022 20:18
This reduces the complexity of the comparison and gets rid of the heap
allocation on every match.

Benchmark before/after (`go test -bench . -benchtime 3s -count 5 -benchmem`):
```
name                old time/op    new time/op    delta
Matches/success-32    66.9ns ± 1%    15.7ns ± 0%   -76.47%  (p=0.016 n=5+4)
Matches/fail-32       27.9ns ± 0%    12.4ns ± 2%   -55.70%  (p=0.016 n=4+5)

name                old alloc/op   new alloc/op   delta
Matches/success-32     5.00B ± 0%     0.00B       -100.00%  (p=0.008 n=5+5)
Matches/fail-32        0.00B          0.00B           ~     (all equal)

name                old allocs/op  new allocs/op  delta
Matches/success-32      1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Matches/fail-32         0.00           0.00           ~     (all equal)
```
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Oct 2, 2022

NB: I could imagine the map from string to tea.Key being defined in the bubbletea package directly. If you prefer that, let me know.

@muesli
Copy link
Copy Markdown
Contributor

muesli commented Oct 5, 2022

NB: I could imagine the map from string to tea.Key being defined in the bubbletea package directly. If you prefer that, let me know.

I think that makes sense and is probably nicer next to the other tea.Key code.

@maaslalani
Copy link
Copy Markdown
Contributor

Whoa! This is awesome @knz, solid optimization!

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Oct 6, 2022

I think that makes sense and is probably nicer next to the other tea.Key code.

Ok see charmbracelet/bubbletea#491

Then to merge this PR I'll need a release number for the go.mod update.

@maaslalani
Copy link
Copy Markdown
Contributor

@muesli Is this good to merge?

@caarlos0 caarlos0 added the enhancement New feature or request label Aug 13, 2024
@caarlos0 caarlos0 changed the title key: optimize Matches perf(key): optimize Matches Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

key: string-based parsing is inefficient

4 participants