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

perf: replace $locator->getClassname() with findQualifiedNameFromPath() #8012

Merged
merged 5 commits into from
Oct 16, 2023

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Oct 6, 2023

Needs #8010

Description
To be honest, I am not sure if the speed has increased, but memory consumption has definitely decreased.

Welcome page:
Before:
Screenshot 2023-10-06 12 59 17
After:
Screenshot 2023-10-06 13 00 15

macOS 12.7

$ php -v
PHP 8.1.24 (cli) (built: Sep 28 2023 22:29:09) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.24, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.24, Copyright (c), by Zend Technologies

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the 4.5 label Oct 6, 2023
@kenjis kenjis marked this pull request as draft October 6, 2023 03:53
@kenjis kenjis added the enhancement PRs that improve existing functionalities label Oct 6, 2023
@neznaika0
Copy link
Contributor

MINGW64 /e/www/ci-demo (dev)
$ php -v
PHP 8.2.11 (cli) (built: Sep 26 2023 15:25:31) (ZTS Visual C++ 2019 x64)
Copyright (c) The PHP Group
Zend Engine v4.2.11, Copyright (c) Zend Technologies

Performance has not changed (my project)

@kenjis
Copy link
Member Author

kenjis commented Oct 6, 2023

See my results. The change is only about 0.5MB.
In real projects, we use more memory. So it can't measure the change.
Try the default Welcome page or Hello World.

@neznaika0
Copy link
Contributor

I tried my test project. Used mem ~7.5 - 7.6 with pagination, filters, events

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Looks good; awaiting rebase.

@kenjis kenjis force-pushed the perf-replace-locator-getClassname branch from 83566ae to d3b5c17 Compare October 13, 2023 08:55
@kenjis kenjis marked this pull request as ready for review October 13, 2023 08:56
@kenjis
Copy link
Member Author

kenjis commented Oct 13, 2023

The difference is not as great as before.

4.5 branch: 1.830 MB
   This PR: 1.621 MB

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

CodeIgniter's niche is being a full-featured monolith framework that is conservative on magic and very lightweight and easy to install. I think anything that keeps our performance impact down will only help the marketing.

@kenjis kenjis merged commit d5ad48e into codeigniter4:4.5 Oct 16, 2023
52 checks passed
@kenjis kenjis deleted the perf-replace-locator-getClassname branch October 16, 2023 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.5 enhancement PRs that improve existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants