Skip to content

Memoize expensive calls to docComments.#458

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
pyguerder:memoize-doc-calls
Feb 26, 2021
Merged

Memoize expensive calls to docComments.#458
ondrejmirtes merged 1 commit intophpstan:masterfrom
pyguerder:memoize-doc-calls

Conversation

@pyguerder
Copy link
Copy Markdown
Contributor

I ran multiple tests using Blackfire. Memoizing these expensive calls reduces both RAM consumption & execution time.

This is an example with running on https://github.com/pyguerder/phpstan-propel2-example/blob/master/generated-classes/App/AnalyzeMe2.php only: https://blackfire.io/profiles/compare/7f25525d-0676-40b8-a809-a92692106be6/graph

And this is an example running on my main project (154 classes): https://blackfire.io/profiles/compare/7cc0ea82-3727-4a95-b948-112678bc0991/graph

@ondrejmirtes can you please have a look at this?

Thanks in advance, best regards

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Hi, this looks promising, thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like to use a control structure (an if) here instead of just ?? on a single line. Like this:

if ($this->reflectionDocComment !== null) {
    $docComment = $this->reflectionDocComment;
} else {
    $docComment = $this->reflection->getDocComment();
    $this->reflectionDocComment = $docComment;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And the same here.

This reduces both RAM consumption & execution time.
@pyguerder
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes thanks for your feedback! Can you please look at it again?

I don't understand why some tests fail. I'm not sure it is related to my changes... Can you please advise?

@ondrejmirtes
Copy link
Copy Markdown
Member

Master is broken, it's not your fault 😊

@ondrejmirtes ondrejmirtes merged commit dfcb87e into phpstan:master Feb 26, 2021
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

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.

2 participants