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

Profiler: measure execution time from the beginning. #32754

Merged
merged 1 commit into from Jun 1, 2023
Merged

Profiler: measure execution time from the beginning. #32754

merged 1 commit into from Jun 1, 2023

Conversation

lmeyer1
Copy link
Contributor

@lmeyer1 lmeyer1 commented May 30, 2023 •
edited

Questions Answers
Branch? develop
Description? Currently the Profiler measures execution time since the call to Profiler::__cunstruct(). We need a beter estimate. If the server exposes the variable $_SERVER['REQUEST_TIME_FLOAT'], use it, else use the $start_time from

PrestaShop/config/config.inc.php

Line 38 in 2aaa49f

$start_time = microtime(true);
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
How to test? Look at #32753 to see the changes
Fixed ticket? Fixes #32753
Related PRs none
Sponsor company Société Bilique de Genève

@lmeyer1 lmeyer1 requested a review from a team as a code owner May 30, 2023 14:02
@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix labels May 30, 2023
matthieu-rolland
matthieu-rolland approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

Nice improvement 👍

I didn't know about REQUEST_TIME_FLOAT

boherm
boherm approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @lmeyer1! :)

@boherm boherm added the Waiting for QA Status: action required, waiting for test feedback label May 30, 2023
@kpodemski
Copy link
Contributor

I think it could be tested by a dev. WDYT @PrestaShop/qa-functional?

@hibatallahAouadni
Copy link
Contributor

Hello @lmeyer1

Thanks for your PR 🚀
We need to wait the maintainers approval on the related issue to test the PR.

Thanks for your patience 🙏

@hibatallahAouadni hibatallahAouadni added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels May 31, 2023
@kpodemski
Copy link
Contributor

@hibatallahAouadni I think we could agree that maintainers reviewing PR is equal to the issue being accepted.

@kpodemski kpodemski added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels May 31, 2023
@hibatallahAouadni hibatallahAouadni self-assigned this Jun 1, 2023
hibatallahAouadni
hibatallahAouadni approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@hibatallahAouadni hibatallahAouadni left a comment

Choose a reason for hiding this comment

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

Hello @lmeyer1

LGTM, QA ✅
image

Thanks!

@hibatallahAouadni hibatallahAouadni added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Jun 1, 2023
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@Progi1984 Progi1984 added this to the 9.0.0 milestone Jun 1, 2023
@nicosomb nicosomb merged commit e2ff7f4 into PrestaShop:develop Jun 1, 2023
14 checks passed
@nicosomb
Copy link
Contributor

nicosomb commented Jun 1, 2023

Thank you @lmeyer1 !

@lmeyer1
Copy link
Contributor Author

lmeyer1 commented Jun 1, 2023

Thank you @nicosomb @hibatallahAouadni @kpodemski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthieu-rolland matthieu-rolland matthieu-rolland approved these changes

@boherm boherm boherm approved these changes

@hibatallahAouadni hibatallahAouadni hibatallahAouadni approved these changes

@M0rgan01 M0rgan01 M0rgan01 approved these changes

Assignees

@hibatallahAouadni hibatallahAouadni

Labels
Bug fix Type: Bug fix develop Branch QA ✔️ Status: check done, code approved
Projects
PR Dashboard
Archived in project
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

Profiler shows incorrect execution time
9 participants
@lmeyer1 @kpodemski @hibatallahAouadni @prestonBot @nicosomb @matthieu-rolland @boherm @M0rgan01 @Progi1984