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

Minor speed improvement #390

Closed

Conversation

etiennebacher
Copy link

Minor improvement: use tabulate() instead of table().

table() returns a named vector and tabulate() returns an unnamed vector, but that doesn't matter here because you remove the names anyway:

set.seed(123)
foo <- sample(1:100, 1e7, TRUE)
bench::mark(
    tabulate(foo),
    table(foo),
    iterations = 20,
    check = FALSE
)
#> Warning: Some expressions had a GC in every iteration; so filtering is
#> disabled.
#> # A tibble: 2 × 6
#>   expression         min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>    <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 tabulate(foo)   12.5ms   13.5ms     72.9     12.8KB     0   
#> 2 table(foo)     551.6ms  649.4ms      1.54   662.2MB     7.86

Created on 2023-02-22 with reprex v2.0.2

I ran the tests locally and everything looked fine.

@etiennebacher
Copy link
Author

There a bunch of conflicts now and I don't want to resolve them as the change is very simple so I'm closing this

@etiennebacher etiennebacher deleted the test-speed-improvement branch February 13, 2024 14:25
@lrberge
Copy link
Owner

lrberge commented Feb 14, 2024

Very sorry Etienne for the huge delay. It's that I had to check it was OK. It indeed was and it was trivial to check, sorry.
Now I added your change in the package, thanks.

@etiennebacher
Copy link
Author

No worries at all, I can't imagine the amount of work behind this package, and it's completely fine to prioritize academic projects (among other things). Thanks once again for this package, it's so convenient and nice to use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants