perf(handler) do not use cache with dbless when looking up service for route #8972
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
bungle
force-pushed
the
perf/no-cache-dbless-get-service
branch
from
dd6e570
to
2de9bc9
Compare
🚀 Performance test resultTest Suite: 01-rps 02-flamegraph (baseline,single_route,simple) [" Click to expandDownload Artifacts for detailed results and interactive SVG flamegraphs. |
…r route ### Summary It does not make sense to use cache with dbless where the data is already in memory, especially because the route rebuild process can use a lookup table. It also consumes a lot of memory to have service in: 1. lmdb (in memory) 2. shared dictionary (in memory) 3. worker local memory So this commit removes the unneccessary use of cache here.
bungle
force-pushed
the
perf/no-cache-dbless-get-service
branch
from
2de9bc9
to
63d9f45
Compare
🚀 Performance test resultTest Suite: 01-rps 02-flamegraph (baseline,single_route,simple) [" Click to expandDownload Artifacts for detailed results and interactive SVG flamegraphs. |
kikito
approved these changes
Jun 30, 2022
There was a problem hiding this comment.
This only affects the loading of services when building the (old) router on dbless and if I understood this comment correctly:
the route rebuild process can use a lookup table
We are using some other kind of cache (lookup table) for the services when rebuilding the Router. That would mean that this change would only affect the initial building of the router.
I spent some time trying to find now build_router changed in the flamegraphs, but could not find it - is this expected?
In principle I am not opposed to this, so I give it a +1.
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.
Summary
It does not make sense to use cache with dbless where the data is already in memory, especially because the route rebuild process can use a lookup table.
It also consumes a lot of memory to have service in:
So this commit removes the unneccessary use of cache here.
Reading shdict also causes locking.