feat: unconditionally send off a final report on LocalReporter drop#125
Open
jlizen wants to merge 3 commits intoasync-profiler:mainfrom
Open
feat: unconditionally send off a final report on LocalReporter drop#125jlizen wants to merge 3 commits intoasync-profiler:mainfrom
jlizen wants to merge 3 commits intoasync-profiler:mainfrom
Conversation
Adds a synchronous report_blocking method with a default no-op that logs a warning. LocalReporter overrides it to copy JFR files via std::fs::copy, and MultiReporter delegates to its children. The method accepts a &Path instead of Vec<u8> so that reporters decide whether and how to read the file, avoiding an unconditional read for reporters that don't support synchronous reporting.
This file contains hidden or 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
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.
📬 Issue #, if available:
Closes #113
✍️ Description of changes:
We want the local report to unconditionally flush any JFRs prior to shutting down the runtime. And then, for all other reporters, existing behavior of warning should be preserved.
I added a
report_blockingmethod with a default implementation to non-op. That default is slightly better than the old one, since we no longer warn EVERY drop if no JFRs are omitted (previously, we unconditionally warned).And then, the LocalReport does what you'd expect. As does the multi-reporter, it attempts each internal report's
warncall on drop.For non-local reporter usage, adds about 1 byte of memory overhead per profiler instance between the
completed_normallybool + padding (which lets us improve the warning). And then, there is an extra 8 bytes for the new vtable entry, and an extra vtable pointer chase on the cancellation path.On the non-local-reporter path (e.g. S3 reporter hitting the default no-op),
try_final_reportdoes:/proc/self/fd/NPathBuf from a format string (~20 byte allocation)statsyscall to check file sizeWe could get that further down by leaving off
ReportMetadata, which we don't really use in the local reporterreport_blockinganyway, but it seemed like a good idea to bake in.🔏 By submitting this pull request