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

Drop old debug globals and configs #16448

Merged
merged 7 commits into from
Feb 12, 2024

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -

@cconard96 cconard96 marked this pull request as draft January 27, 2024 20:08
@cconard96 cconard96 marked this pull request as ready for review January 27, 2024 23:27
src/XHProf.php Outdated Show resolved Hide resolved
src/Api/API.php Outdated Show resolved Hide resolved
@cedric-anne
Copy link
Member

I simplified a bit the code by computing durations without using the Profiler. There is no need to add complexity to the Profiler usage to just make a $start_time - $end_time calculation.

@cconard96
Copy link
Contributor Author

The ability to use the profiler as a regular timer should stay I think. There is at least one specific use case where it is better than manually computing
Currently, dashboard cards show a duration when in debug mode but it only covers one portion of the work done to load them. With the profiler, we can start a timer in the AJAX code, and then retrieve the duration at the time the HTML is generated.

@cedric-anne
Copy link
Member

The ability to use the profiler as a regular timer should stay I think. There is at least one specific use case where it is better than manually computing Currently, dashboard cards show a duration when in debug mode but it only covers one portion of the work done to load them. With the profiler, we can start a timer in the AJAX code, and then retrieve the duration at the time the HTML is generated.

The problem here was that the Profiler was used outside the debug mode, and it was necessary to introduce some logic to bypass existing limitations, eg the $force_start param:

    /**
     * @param bool $force_start If true, the section will be started even if the user is not in debug mode.
     *                          This is useful if the profiler is going to be used to simply time a section of code and
     *                          the result will be used in the application.`
     */

I am not against giving the ability to get the execution time of a given section if it is easy to use for the developper and does not introduce too much complexity.
If it permits to enhance the dashboard profiling, then this change would be welcome, but please do it in another PR to simplify reviews. It would probably not conflicts with current changes.

@cedric-anne cedric-anne added this to the 10.1.0 milestone Feb 7, 2024
@cedric-anne cedric-anne merged commit b699bc6 into glpi-project:main Feb 12, 2024
8 checks passed
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.

4 participants