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

Save CPU time resources on DateTimeZone creation #213

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

pvgnd
Copy link
Contributor

@pvgnd pvgnd commented Apr 28, 2020

Introduces a DateTimeZoneFactory to keep references of already instatiated DateTimeZone.

Benchmark used:

<?php
require 'vendor/autoload.php';
$holidays = Yasumi\Yasumi::create('France', 2019);

Run on develop branch :
blackfire run --samples=20 php benchmark.php

Profiling: [########################################] 20/20
Blackfire Run completed
Graph URL https://blackfire.io/profiles/bc72f011-ad87-4c92-8472-ea90ef178aa3/graph
No tests! Create some now https://blackfire.io/docs/cookbooks/tests
No recommendations

I/O Wait     1.49ms
CPU Time     13.1ms
Memory        614KB
Wall Time    14.6ms
Network         n/a     n/a     n/a
SQL             n/a     n/a

Run on PR branch :
blackfire run --samples=20 php benchmark.php

Profiling: [########################################] 20/20
Blackfire Run completed
Graph URL https://blackfire.io/profiles/7fc3116c-358c-4213-8435-c7676e84ea54/graph
No tests! Create some now https://blackfire.io/docs/cookbooks/tests
No recommendations

Wall Time    11.1ms
I/O Wait      632µs
CPU Time     10.5ms
Memory        616KB
Network         n/a     n/a     n/a
SQL             n/a     n/a

image
https://blackfire.io/profiles/compare/d1d4c4ea-b553-4738-aba1-851351a01ebe/graph

@stelgenhof
Copy link
Member

Thank you @pvgnd ! Looks like a good improvement. I can't access those links however.
Anyway I'll check your PR in the next coming days.

Cheers! Sacha

@pvgnd
Copy link
Contributor Author

pvgnd commented Apr 28, 2020

@stelgenhof I just made blackfire links public

@stelgenhof
Copy link
Member

Thanks @pvgnd Was able to view the profile. Very insightful.

@stelgenhof stelgenhof self-requested a review April 29, 2020 10:33
@stelgenhof stelgenhof added this to the v2.3.0 milestone Apr 29, 2020
Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked your PR and looks good to me!

@stelgenhof stelgenhof merged commit 51aed0e into azuyalabs:develop Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants