Improve profiling output to make storage performance easier to understand #2059
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
I couldn't figure out why there was so much complication around distinguish the kinds of batches. I think it wasn't really necessary. This simplifies the code a bit. Signed-off-by: Ben Pfaff <blp@feldera.com>
I found this test confusing because it inserted a couple of batches that should get merged but asserted that they hadn't been merged yet. Signed-off-by: Ben Pfaff <blp@feldera.com>
A lot of the nodes in the diagrams I look at are wider than otherwise necessary, which in turn makes the diagrams themselves wider than otherwise necessary, because they have the string `database-stream-processor/src/` and other long directory names in them. This gets rid of that by replacing each directory name in the nodes by just its first letter, which is enough to make the file name unambiguous in practice. Signed-off-by: Ben Pfaff <blp@feldera.com>
This makes it easier to understand what's going on with a spine while looking at a profile. Signed-off-by: Ben Pfaff <blp@feldera.com>
blp
added
enhancement
gz
approved these changes
Jul 17, 2024
There was a problem hiding this comment.
lgtm, fyi we already have a metric for merge reduction too https://github.com/feldera/feldera/blob/main/crates/dbsp/src/trace/spine_async/mod.rs#L613
Oh, that's good to know (it's per-process though instead of per-spine). I hadn't taken proper notice of those metrics. |
ryzhyk
approved these changes
Jul 17, 2024
|
we could make them per-spine with lables so they can be aggregates.. long term maybe it's worth doing it like this rather than having to maintain our own profiler |
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.
@gz, this modifies the async spine code, mostly to simplify it a bit, but also to add some more profiling output to make it easier to understand what's happening storage-wise.
@ryzhyk, I changed the profiling output a bit to shorten paths because that makes the boxes a lot smaller and therefore it's easier to read the profiles. Would like your OK on that.
Is this a user-visible change (yes/no): no