Collect stream statistics and offer new hook to check in on them #1767
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
bbannier
reviewed
Jun 21, 2024
bbannier
reviewed
Jun 21, 2024
rsmmr
force-pushed
the
topic/robin/gh-3779-stream-stats
branch
from
733b5a4
to
3cbad6b
Compare
|
Pushed a new version that introduces a property |
We implement this by storing the current input stream inside the internal parse struct as a new field `__stream`. Using `stream()` then turns into an access to that field (i.e, `*(self.__stream)`). If that field never gets accessed, the optimizer will remove it, so there's no overhead if nobody ever uses `stream()` with a unit.
rsmmr
force-pushed
the
topic/robin/gh-3779-stream-stats
branch
from
3cbad6b
to
eb76180
Compare
|
Ready for review now, now with |
bbannier
previously approved these changes
Jun 24, 2024
This returns a struct of the following type, reflecting the input
seen so far:
```
type StreamStatistics = struct {
num_data_bytes: uint64; ## number of data bytes processed
num_data_chunks: uint64; ## number of data chunks processed, excluding empty chunks
num_gap_bytes: uint64; ## number of gap bytes processed
num_gap_chunks: uint64; ## number of gap chunks processed, excluding empty chunks
};
```
This adds support for a new unit hook:
```
on %sync_advance(offset: uint64) {
...
}
```
This hook is called regularly (see below) during error recovery when
synchronization skips over data or gaps while searching for a valid
synchronization point. It can be used to check in on the
synchronization to, e.g., abort further processing if it just keeps
failing. `offset` is the current position inside the input stream that
synchronization just skipped to.
By default, "called regularly" means that it's called every 4KB of
input skipped over while searching for a synchronization point. That
value can be changed by setting a unit property
`%sync-advance-block-size = <number of bytes>`.
As an additional minor tweak, this also changes the name of what used
to be the `__gap__` profiler to now be called `__sync_advance` because
it's profiling the time spent in skipping data, not just gaps.
Implementation notes:
1. I considered reusing `on %gap()` instead of adding a new hook, but
it turns out the semantics are different than what I think we want
here: (1) it doesn't trigger for non-gap data (by definition), so
couldn't check in on long intervals of searching through payload
data; and (2) it's called when a gap added to the input, not when
it's encountered.
2. The new `ASTInfo` inside the Spicy code generator might seem
overkill for just the one field it currently contains. However, I'm
planing to use this for other information in the future as well
(specifically, tracking the need for `&on-heap`).
Closes #3779.
rsmmr
force-pushed
the
topic/robin/gh-3779-stream-stats
branch
from
e575b7c
to
fe832f2
Compare
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.
This extends the
streamtype to track statistics about their data, and it addsnew
%sync_advancehook that can be used to check statistics duringsynchronization. For more information, see the individual commits.
Closes #1768.