VimPerformanceState#vim_performance_state_for_ts optimizations #22611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 tasks
|
update:
|
|
update:
|
defaulting to passing time into the method. Places calling it, ensure that Fixing: sometimes we were passing non iso8061 into the method so it was not finding the cached VimPerformanceState records | ms | bytes | objects |query | qry ms | rows |` comments` | ---:| ---:| ---:| ---:| ---:| ---:| --- | 1,699.1 | 19,618,021* | 283,526 | 9 | 1,519.6 | 79 |`before` | 1,664.1 | 15,672,327* | 225,826 | 5 | 1,550.8 | 75 |`after-fetch-time`
wanted to create specs around vim_performance_state_association and hit a tangent. This already cached newly created vim_performance_state records when one did not exist in the db but it never looked for that value in the local cache. now it looks that value up
since perf_capture_state does a find for now, not reason to try and find it first have tried to remove this extra query a few times. previous attempts deleted this line. this one is different as we added the unless. there were a few cases that we do want a find here. (luckily there are tests around this)
|
Checked commits kbrock/manageiq@0d34281~...e6268fc with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
|
update:
|
3 tasks
Fryguy
reviewed
Aug 1, 2023
| state = vim_performance_states.detect { |s| s.timestamp == t } | ||
| end | ||
| # look for state from previous perf_capture_state call | ||
| state ||= @states_by_ts[ts_iso_now] |
There was a problem hiding this comment.
this part is confusing to me. The right hand side will only execute if state is nil which means it would have ran the previous if state.nil? block. That block fetches a state using ts_iso_now. That means this line can only be run if it couldn't get a state using ts_iso_now, but then we use ts_iso_now to lookup into the @states_by_ts?
|
Backported to |
Fryguy
added a commit
that referenced
this pull request
Aug 2, 2023
VimPerformanceState#vim_performance_state_for_ts optimizations (cherry picked from commit 1a00c40)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
commits:
VimPerformanceState.nowbefore callingperf_capture_state. (capture also does this query)@paramdocumentation for someTimevalues passed around