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

Make UI tab completer list use Vec<&str> instead of Vec<String> #53

Closed
fsktom opened this issue Jul 11, 2023 · 4 comments
Closed

Make UI tab completer list use Vec<&str> instead of Vec<String> #53

fsktom opened this issue Jul 11, 2023 · 4 comments
Assignees
Labels
cli Regarding `endsong_ui` crate question Further information is requested refactor

Comments

@fsktom
Copy link
Owner

fsktom commented Jul 11, 2023

Don't know how feasible this is or if it's necessary.

See 275b337

@fsktom fsktom added question Further information is requested refactor labels Jul 11, 2023
@fsktom fsktom self-assigned this Jul 11, 2023
@fsktom
Copy link
Owner Author

fsktom commented Jul 11, 2023

Actually internally, rustyline prefers &str
https://docs.rs/rustyline/latest/rustyline/completion/trait.Completer.html
https://docs.rs/rustyline/latest/rustyline/completion/trait.Candidate.html

but I'd have to figure it out with the lifetimes somehow
seems quite painful

@fsktom
Copy link
Owner Author

fsktom commented Jul 12, 2023

The problem is that SongEntries.artists() is pretty slow
in cargo bench it takes 2.5ms while albums(&asp) takes below a ms
I figured it may be due to allocations of Strings but idk

@fsktom fsktom added the cli Regarding `endsong_ui` crate label Jul 27, 2023
@fsktom
Copy link
Owner Author

fsktom commented Aug 5, 2023

Well.... turns out I basically did this automatically with #62 xd
And the problem of dealing with lifetimes solved itself by using Rc :D

@fsktom
Copy link
Owner Author

fsktom commented Aug 5, 2023

Performance improvements in 21bf2b3
From

artists_vec             time:   [2.7070 ms 2.7476 ms 2.7971 ms]
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe

albums_vec              time:   [255.43 µs 261.76 µs 269.15 µs]
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) high mild
  4 (4.00%) high severe

to

artists_vec             time:   [1.5549 ms 1.5785 ms 1.6062 ms]
                        change: [-44.196% -43.033% -41.859%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe

albums_vec              time:   [202.15 µs 203.30 µs 204.42 µs]
                        change: [-22.566% -21.586% -20.714%] (p = 0.00 < 0.05)
                        Performance has improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Regarding `endsong_ui` crate question Further information is requested refactor
Projects
None yet
Development

No branches or pull requests

1 participant