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.

Sign up for GitHub

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

Jump to bottom

Performance investigation #4026

Closed
staabm opened this issue Oct 11, 2018 · 6 comments
Closed

Performance investigation #4026

staabm opened this issue Oct 11, 2018 · 6 comments
Labels
kind/enhancement kind/meta Set of actions to be broken down into single issues/PR's

Comments

@staabm
Copy link
Contributor

staabm commented Oct 11, 2018 •
edited

while crunching together initial numbers in #4024 (comment) I did some blackfire profiles

my findings:

linting a single file:
blackfire run php ./PHP-CS-Fixer/php-cs-fixer fix ./sabre-dav/lib/CalDAV/Calendar.php

https://blackfire.io/profiles/739e8c50-2345-4f4b-a157-e94b9b220029/graph
grafik

=> PhpCsFixer\Tokenizer\Token::equals() and PhpCsFixer\Tokenizer\Token::isGivenKind() seem to be slow

linting a folder of files (pretty similar picture):
blackfire run php ./PHP-CS-Fixer/php-cs-fixer fix ./sabre-dav/lib/CalDAV/

https://blackfire.io/profiles/7db4aa88-ece8-4fd4-9850-a1a7467bf1a4/graph
grafik

=> PhpCsFixer\Tokenizer\Token::equals(), PhpCsFixer\Tokenizer\Tokens::extractTokenKind(), PhpCsFixer\Tokenizer\Tokens::offsetSet() seem to be slow
@keradus
Copy link
Member

keradus commented Oct 12, 2018 •
edited

=> PhpCsFixer\Tokenizer\Token::equals() and PhpCsFixer\Tokenizer\Token::isGivenKind() seem to be slow

can you elaborate on that ?
what I can see is that they are called plenty of times, and well, they will, due to their responsibility

@staabm
Copy link
Contributor Author

staabm commented Oct 12, 2018

can you elaborate on that ?
what I can see is that they are called plenty of times, and well, they will, due to their responsibility

those methods are called so frequently that very minor changes can lead to walltime speed win. alternatively one could check the calling locations whether some of those calls can be saved, e.g. by using temp-variables or similar

This was referenced Oct 14, 2018
Closed
Closed
@SpacePossum SpacePossum added kind/enhancement kind/meta Set of actions to be broken down into single issues/PR's labels Oct 15, 2018
This was referenced Oct 16, 2018
Merged
Merged
keradus added a commit that referenced this issue Nov 28, 2018
@keradus
minor #4047 Token - inline $other->getPrototype() to speedup equals()…
e7bb262 
… (staabm, keradus)

This PR was merged into the 2.12 branch.

Discussion
----------

Token - inline $other->getPrototype() to speedup equals()

as measured in #4026 we try to speedup equals() by inlinening the $this->getPrototype() call.

~~also we use FQCN for native functions, because equals() is called a lot.~~

this speedsup the process by 4-5%.

see https://blackfire.io/profiles/compare/f4eefae8-6d09-4ab4-a833-f828d74a7b52/graph
![image](https://user-images.githubusercontent.com/120441/46916011-d6d2c400-cfb4-11e8-89e1-ec64e77477f1.png)

Commits
-------

1aecb36 Token - inline $other->getPrototype() to speedup equals()
keradus added a commit that referenced this issue Nov 28, 2018
@keradus
minor #4048 Tokens - inlined extractTokenKind() call on the hot path …
49c3a9e 
…(staabm)

This PR was merged into the 2.12 branch.

Discussion
----------

Tokens - inlined extractTokenKind() call on the hot path

as identified in #4026 we inline calls to `extractTokenKind()` which leads to a 5-7% speed increase.

most calls of this method happen thru the changed code as can be seen in the blackfire profile

![image](https://user-images.githubusercontent.com/120441/46915856-1a77fe80-cfb2-11e8-9e0c-531b88059ad4.png)

Commits
-------

27c78e3 Tokens - inlined extractTokenKind() call on the hot path
@keradus
Copy link
Member

keradus commented Nov 28, 2018

Great work @staabm !
Closing due to merge of #4046, #4047, #4048 .

After saying that, if anyone has idea how to improve performance even further, PRs are welcome !

@keradus keradus closed this as completed Nov 28, 2018
@staabm
Copy link
Contributor Author

staabm commented Nov 28, 2018

Thx for merging. I am very proud to contribute to this great tool.

@keradus
Copy link
Member

keradus commented Nov 28, 2018

Thanks for your patience as well ;)

@ntzm
Copy link
Contributor

ntzm commented Nov 28, 2018

Awesome work on this @staabm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/enhancement kind/meta Set of actions to be broken down into single issues/PR's
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@staabm @keradus @ntzm @SpacePossum