Conversation
|
Web viewer built successfully.
View image diff on kitdiff. Note: This comment is updated whenever you push a commit. |
|
Reposting my comment - this PR really brings a massive improvement to viewing text log. As you mentioned in the PR description, I would love multiline logs but the text viewer is now usable. I'll take that over unusable! I have tested on my rrd file containing 5 million lines. fwiw we don't want people viewing this many lines at once (would prefer smaller time segments) but people will try anyway. 
|
| /// | ||
| /// Rows without a level always pass. | ||
| #[derive(Clone, PartialEq, Eq)] | ||
| pub struct LevelFilter(pub BTreeSet<String>); |
There was a problem hiding this comment.
This is out of scope for this PR and I should probably submit a feature request, but what do you think about extending the filters beyond levels, specifically to filtering text? It looks like the framework you have here with the LevelFilter could be extended to also filter specific parts of the text, something like
pub struct BodyFilter {
include: FilterMatcher,
exclude: FilterMatcher,
}
There was a problem hiding this comment.
Think that's a good feature request indeed!
Source-Ref: 369957f70b98554dcd7fc0d454ea8f63eca58095
Source-Ref: dcadf7520625dbdc1204ee662218e0c10c29b571
Source-Ref: a4855174f2fe91be24df1f12b94b19a375d9e939
Source-Ref: a91a74eb95b09055a06609539663480554002079
Source-Ref: 141066893b290c8a36e4fa7cbeb14a636e54517f
Source-Ref: 27a09af7347c7d9b99d513c97f534e7164f618af
Source-Ref: 5a69cdcfba304bad69038a66651b3de1f7fc6aee
Source-Ref: a7339535b1da90f82ebfdb527b7a5f30c7eae541
Source-Ref: 3b23e59e90cd2d5f586cc834ae043d0516b7557e
Source-Ref: 7f2db9c6e91eecadf599667911d3b5d4afbefb8c
6fa6e84 to
1d124db
Compare
Wumpf
left a comment
There was a problem hiding this comment.
I meant to take this PR over but didn't get to it. Now that I do I see too many things that need fixing unfortunately, I think we should start this over.
The main rub point is that this violates our datamodel and bypasses regular range queries: we suddenly require all text log properties to be on the same row in a chunk; this happens in reality often but is insanely brittle as e.g. rrd optimize may distribute properties of different columns onto arbitrary rows.
This then leads to a lot of heavy handed range-query emulation instead of just doing regular range queries.
At the same time we're not really gaining much from that since we're still depending on deep inspection of all chunks
I strongly suspect that the real wins here are just the fact that we use egui_table instead of egui-extras. The later has to know and layout all text logs in existance whereas the former only shows what's on screen at a given point in time.
What I believe we should do instead as a first step is to stick to our normal range queries in the visualizer but then emit elements only for the visible text logs and tell an egui_table what to show and where its scrollbar has to be.
This PR winds up doing exactly that but builds up a complex separate datastructure for that - in fact very similar to what our range query cache already does!
Meaning to say I strongly suspect we can have almost all the gains with a lot less code and, most importantly, without creating a new special case in the datamodel
…ance # Conflicts: # rerun/crates/views/re_view_text_log/src/row_layout.rs # rerun/crates/views/re_view_text_log/tests/perf.rs # rerun/crates/views/re_view_text_log/tests/snapshots/text_log_level_filter.png # rerun/crates/views/re_view_text_log/tests/snapshots/text_log_view_default_level.png Source-Ref: f50e9665f5224ce6deaf55d985d5114db2190055
Related
What
Speed up the TextLog view by only querying the view range of text logs instead of EVERYTHING!
How
Previously the visualizer did a range query for EVERYTHING and materialized every textlog entry on each frame, just to figure out how many rows it had and where to scroll to.
This scales linearly with the amount of
TextLogs logged! 😱 This is fine for a few text logs, but the viewer grinds down to 5fps for recordings with 500k lines (#7562).The fix is to stop querying the actual data to create the view layout and instead create the layout using chunk metadata. Chunks already know their time range and
TextLogcounts, so that's enough to derive the total row count and which rows fall where in time. This lets us only query the data for the currently visible time range.Additionally this replaces the view's current
egui_extras::TableBuilderrendering withegui_tablewhich supports virtual scrolling over a known number of rows (we already do this for the dataframe view!).Filtering by log levels through the blueprint is a bit complicated, since it requires log level component information from inside each chunk. But since chunks are immutable (right?), we can get around this by scanning a chunk once and caching the per-level row counts by
ChunkId.This new approach also doesn't support rendering multi line logs, the data is shown when hovering. Implementing this makes the accounting a lot more tedious and should be done as a followup.
However, this does mean a
TextLogrow's level/color must come from that row alone, which means blueprint overrides/defaults for them are no longer applied. Also, level/color no longer carry follow the latest-at semantics (which I think is fine for this archetype, and actually desired?)