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

Reduce recursive-call overhead in MetricTimeQueryValidationRule #1440

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

plypaul
Copy link
Contributor

@plypaul plypaul commented Oct 2, 2024

The check in MetricTimeQueryValidationRule is called for every metric in a derived metric's ancestors, so this moves expensive parts of the check to only where it's needed and caches results when possible to reduce runtimes. The signature for the validation rule classes were changed, so there are a number of diff lines related to that.

This is also a common method that is called during query resolution. The memory
used by the cache is small since the number of measures is on the order of ~100
in large manifests.
…n `MetricTimeQueryValidationRule`.

The check in `MetricTimeQueryValidationRule` is called for every metric in a
derived metric's ancestors, so this moves expensive parts of the check to only
where it's needed and caches results when possible.
@cla-bot cla-bot bot added the cla:yes label Oct 2, 2024
Copy link

github-actions bot commented Oct 2, 2024

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@plypaul plypaul marked this pull request as ready for review October 2, 2024 16:23
Base automatically changed from p--short-term-perf--13 to main October 2, 2024 19:44
@plypaul plypaul merged commit df26a6b into main Oct 2, 2024
36 of 38 checks passed
@plypaul plypaul deleted the p--short-term-perf--15 branch October 2, 2024 19:49
courtneyholcomb pushed a commit that referenced this pull request Oct 9, 2024
)

The check in `MetricTimeQueryValidationRule` is called for every metric
in a derived metric's ancestors, so this moves expensive parts of the
check to only where it's needed and caches results when possible to
reduce runtimes. The signature for the validation rule classes were
changed, so there are a number of diff lines related to that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants