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

Optimisation for Varien_Object::_addFullNames #2821

Merged
merged 2 commits into from
Dec 18, 2022

Conversation

AGelzer
Copy link
Contributor

@AGelzer AGelzer commented Dec 17, 2022

Description (*)

Optimization for Varien_Object::_addFullNames

  • less code
  • array_intersect_key with array_flip is faster than array_intersect with array_values
  • avoid array_search
  • Tested with PHP8.0 Openmage 19.4.20 => 3x faster (Test Object: Mage_CatalogInventory_Model_Stock_Item)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

  • Thanks

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* labels Dec 17, 2022
@sreichel
Copy link
Contributor

sreichel commented Dec 17, 2022

Can you please add some information about tests or measurement?

Just did a quick test with xhprof on admin product edit page ... before and after change ...

xhp1

Or ... here it is faster before ... Test

@AGelzer
Copy link
Contributor Author

AGelzer commented Dec 18, 2022

Can you please add some information about tests or measurement?

Just did a quick test with xhprof on admin product edit page ... before and after change ...

xhp1

Or ... here it is faster before ...

Test

Hi Sven ,
Thanks for https://onlinephp.io and testing :)
3x faster refer to function performance.
New is faster: Test

@AGelzer
Copy link
Contributor Author

AGelzer commented Dec 18, 2022

Changes

  • Added empty check for syncFieldsMap

Performance Tests:

Compare: Old/New, Empty/Not empty Map (PHP8.2, PHP8.1, PHP8.0, PHP7.4)

XHProf (system load depend)

  • PHP 8.0.26
  • Processor: 24 x AMD Ryzen 9 5900X 12 Core Processor
  • OpenMage: 19.4.21 (1 x Category, 1 x Product (simple), Cache: activated)

Screenshot 2022-12-18 at 14-55-06 XHProf Hierarchical Profiler Report
Screenshot 2022-12-18 at 14-58-55 XHProf Hierarchical Profiler Report
screenshot-xhprof age local-2022 12 18-14_21_52
Screenshot 2022-12-18 at 15-18-51 XHProf Hierarchical Profiler Report

@sreichel
Copy link
Contributor

Dont know why i had different numbers on phponline yesterday, but LGTM.

@fballiano fballiano merged commit 9b2cdf6 into OpenMage:1.9.4.x Dec 18, 2022
@sreichel sreichel added the performance Performance related label Dec 18, 2022
@AGelzer
Copy link
Contributor Author

AGelzer commented Dec 19, 2022

@sreichel @fballiano thanks :)

@sreichel
scientific notation?

Execution time : 1.3113021850586E-5 seconds
Execution time : 4.0531158447266E-6 seconds

@sreichel
Copy link
Contributor

scientific notation?

Yes. Didn't see it.

Just one question ... did you use some tools for xdebug camparison? (or changed URL manually?)

DDEV provides xhprof support, but only links to current report (if enabled).

Would be nice to get such diff "on click".

(https://github.com/SergeyRock/xhprof-admin)

@AGelzer
Copy link
Contributor Author

AGelzer commented Dec 25, 2022

I used phacility/xhprof with my quick & dirty changes

Screenshot 2022-12-25 at 10-52-54 XHProf Hierarchical Profiler Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* performance Performance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants