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

Use a template free shim around quarterly.h classes #322

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Apr 17, 2023

To greatly improve compilation speed

Basically, quarterly-year-quarter-day.cpp needed every function to be templated on both the precision and the quarter start, which resulted in a massive explosion in the amount of template code that had to be generated.

We don't really need templating on the quarter start, even though using a template in quarterly.h is technically correct (because it is part of the type definition). So this introduces a minimal shim around those classes that re-exposes them in a template free way.

The shim doesn't expose everything from quarterly.h, only what is needed for the R API. We can expose more as needed.

Essentially the template parameter is now a class member quarterly_shim::year. We went:

// from
quarterly::year<start>

// to
quarterly_shim::year.start()

On my 2018 Intel Mac, compilation time went down from 70s to 25s!

The internal switch over the quarter start types does make the quarterly type a little slower for some operations, but I think it is worth it:

library(clock)
set.seed(123)

q <- sample(4, 1e7, TRUE)
x <- year_quarter_day(2019, q)

bench::mark(set_day(x, "last"), iterations = 100)

# Before
#> # A tibble: 1 × 6
#>   expression              min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>         <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 set_day(x, "last")   77.9ms   81.4ms      12.0    38.2MB     8.01

# After
#> # A tibble: 1 × 6
#>   expression              min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>         <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 set_day(x, "last")   99.1ms    103ms      9.51    38.2MB     6.34

Since having a switch in a `constexpr` is a C++14 feature
Locally it doesn't seem like this interferes with `NOEXCEPT`
@DavisVaughan DavisVaughan merged commit 4a8eb23 into main Apr 17, 2023
@DavisVaughan DavisVaughan deleted the feature/yqd-speedup branch April 17, 2023 15:28
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.

1 participant