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

Fix constexpr of gregorian::date::date(special_values) to improve perf #214

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Jul 31, 2022

Fixes #121

Turns out even when all functions without the library are prefixed with constexpr, GCC up to at least v10 does not consider this constructor as constexpr at least when generating code.

BOOST_CXX14_CONSTEXPR explicit date(special_values sv):
      date_time::date<date, gregorian_calendar, date_duration>(date_rep_type::from_special(sv))
    {
      if (sv == min_date_time)
      {
        *this = date(1400, 1, 1);
      }
      if (sv == max_date_time)
      {
        *this = date(9999, 12, 31);
      }
    }

The culprit is the assignment to *this. Refactoring the code to initialize the instance just once improves code generation significantly. Note that clang since at least v11 does not exhibit this problem.

GCC up to at least 10.2 fail to resolve
gregorian::date::date(special_values) as constexpr function due to
assignment to *this within the constructor. Refactoring constructor to
initialize the instance once leads to large performance improvement.
@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Merging #214 (a8478d4) into develop (f972dc9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #214   +/-   ##
========================================
  Coverage    91.74%   91.75%           
========================================
  Files           77       77           
  Lines         4823     4827    +4     
========================================
+ Hits          4425     4429    +4     
  Misses         398      398           
Impacted Files Coverage Δ
include/boost/date_time/gregorian/greg_date.hpp 100.00% <100.00%> (ø)
...boost/date_time/posix_time/posix_time_duration.hpp 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f972dc9...a8478d4. Read the comment docs.

@JeffGarland JeffGarland merged commit b81ed28 into boostorg:develop Aug 1, 2022
@JeffGarland
Copy link
Collaborator

Looks good - thanks!

JeffGarland added a commit that referenced this pull request Oct 31, 2022
* fix typo in time_period docs (#212)

Co-authored-by: Quinn O'Connor <qoconnor@fastenal.com>

* Fix constexpr of gregorian::date::date(special_values) to improve perf (#214)

GCC up to at least 10.2 fail to resolve
gregorian::date::date(special_values) as constexpr function due to
assignment to *this within the constructor. Refactoring constructor to
initialize the instance once leads to large performance improvement.

* Avoid using likely function as multiple projects define a likely macro (#216)

* Iso doc fixes (#215)

* to_iso_*string() use "." as fractional separator

The fractional separator for the various to_iso_* methods
is "." not "," (per to_iso_string_type() implementation).
Fix the documentation to match the implementation.

* use "ISO 8601" not "iso" in documentation

The standard is "ISO 8601", so use that instead
of just "iso" or "ISO" in comments and documentation.

* fractional seconds only included if non-zero (#110)

Consistently document that the fractional seconds
are only included if non-zero.

Use "where fffffffff" not "were fff".

* Fix ccache saving on cache hit (#211)

See boostorg/boost-ci#166

* chore: bump macos runner version (#213)

GitHub Action is sunsetting the macOS 10.15 Actions runner. It will stop working intermittently until being completely removed by 2022-8-30: https://github.blog/changelog/2022-07-20-github-actions-the-macos-10-15-actions-runner-image-is-being-deprecated-and-will-be-removed-by-8-30-22

Co-authored-by: Quinnsicle <qtoconnor@gmail.com>
Co-authored-by: Quinn O'Connor <qoconnor@fastenal.com>
Co-authored-by: Povilas Kanapickas <povilas@radix.lt>
Co-authored-by: Antony Polukhin <antoshkka@gmail.com>
Co-authored-by: Luke Mewburn <luke@mewburn.net>
Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Co-authored-by: Baoshuo Ren <i@baoshuo.ren>
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.

Default-constructing boost::posix_time::ptime and possibly other types is slow
2 participants