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

[performance] Optimize LocalDate::plusDays(1) calls. #79

Merged
merged 2 commits into from
Sep 16, 2023

Conversation

gnutix
Copy link
Contributor

@gnutix gnutix commented Sep 15, 2023

Fixes #78.

Local PHP script-based benchmark shows that for 10'000'000 iterations of LocalDate::plusDays(1) (from 2023-09-15 to 29402-10-11) takes an average of 25 seconds without this patch, and an average of 5 seconds with it. Adding the patch does not change the results if the input argument of plusDays() is something else than 1 (or only a non-noticeable amount).

Benchmark script
<?php # var/script.php

declare(strict_types=1);

namespace Script;

use Brick\DateTime\LocalDate;
use Brick\DateTime\Stopwatch;
use Symfony\Component\ErrorHandler\Debug;

require dirname(__DIR__) . '/config/bootstrap.php';

Debug::enable();

$stopwatch = new Stopwatch();

$date = LocalDate::parse('2023-09-15');

$stopwatch->start();
for ($i = 0; $i < 10_000_000; ++$i) {
  $date = $date->plusDays(2);

  if ($i === 0 || $i === 9_999_999) {
      dump((string) $date);
  }
}
$stopwatch->stop();

dd((string) $stopwatch->getElapsedTime());

I understand this is quite a specific patch, mostly related to our own use case in our code base, but I still think it might benefit users of this library, because looping from day to day seems like quite a common use case (it's done even within the library, in LocalDateRange::getIterator() and LocalDate::plusWeekdays()).

We thought of adding a -1 use case too, but it's way more complicated to implement, and I'd guess it's not as useful/impactful.

Credit goes to @BastienClement.

src/LocalDate.php Show resolved Hide resolved
src/LocalDate.php Show resolved Hide resolved
@BenMorel
Copy link
Member

Looks good! I agree this is a very common operation, and deserves the x5 performance boost this extra code brings.

@gnutix
Copy link
Contributor Author

gnutix commented Sep 16, 2023

(Final Edit): OK, there's no more performance improvement I can think of for now. 😅

Could you please create a new minor release once this PR is merged ? 🙏

@BenMorel BenMorel merged commit f1f432c into brick:master Sep 16, 2023
7 checks passed
@BenMorel
Copy link
Member

@gnutix Released as 0.5.2! Thanks for your contribution.

@gnutix
Copy link
Contributor Author

gnutix commented Sep 17, 2023

Thanks @BenMorel! Would you mind adding @BastienClement in the release note as a contributor (if you can edit it) ? He did most of the thinking behind those 3 PRs. :)

@gnutix gnutix changed the title Optimize performance of LocalDate::plusDays(1) calls. [performance] Optimize LocalDate::plusDays(1) calls. Sep 19, 2023
@BenMorel
Copy link
Member

@gnutix No problem, done!

https://github.com/brick/date-time/releases/tag/0.5.2

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.

[performance] Optimizing LocalDate::plusDays() method
2 participants