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

Avoid unnecessary calls to Profile::get in logger #1200

Merged
2 commits merged into from Mar 31, 2023
Merged

Avoid unnecessary calls to Profile::get in logger #1200

2 commits merged into from Mar 31, 2023

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2023

Purpose

Store logfilter property instead of querying it at every method's call. This will improve performance.

Context

Taken from https://gitlab.rd.nic.fr/zonemaster/zonemaster-engine/-/commit/8b303b6c5d2e1a148c592325839256cc85f828b8

Credits to @blacksponge for the proposal

Changes

Logger.pm

How to test this PR

Tests should pass.

To assess the performance improvements, run:

perl -d:NYTProf -MZonemaster::Engine -E 'say join "\n", Zonemaster::Engine->test_zone("zonemaster.net")'
nytprofhtml --open

And check that the time passed in the _check_filter routine has decreased.

Credits to @blacksponge for the proposal
@ghost ghost added this to the v2023.1 milestone Feb 24, 2023
@ghost ghost requested a review from hannaeko February 24, 2023 14:34
@matsduf
Copy link
Contributor

matsduf commented Feb 26, 2023

Could you please add some documentation to what method _check_filter does? Preferably with POD.

@ghost ghost merged commit 0b0b077 into zonemaster:develop Mar 31, 2023
@ghost ghost deleted the avoid-redudundant-calls branch March 31, 2023 13:10
@hannaeko hannaeko self-assigned this Jun 1, 2023
@hannaeko
Copy link
Member

hannaeko commented Jun 1, 2023

Tests are working. I did not test the performances increase during release testing.

@hannaeko hannaeko added S-ReleaseTested Status: The PR has been successfully tested in release testing V-Patch Versioning: The change gives an update of patch in version. labels Jun 1, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants