feat: per-frame tracing buffer that dumps on panic#1790
Conversation
Replaces the always-on debug-mode file log with a per-frame in-memory buffer that captures tracing events emitted during run_rewrite_passes and dumps them to a $TMPDIR/masonry-*-panic.log file only when the process panics. This eliminates per-log-line I/O on the hot path while preserving full diagnostics for crashes. Closes linebender#1556. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AndroidTraceLayer's Layer impl requires Debug on the inner subscriber types, which transitively requires BufferWriter: Debug. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hi Tony, thanks for the PR. Would you be so kind to describe a bit of the process you went through when making these changes? I'm just wondering if this is a result of Claude's thinking or if you played any role in it too. Basically, what differentiates it from a simple Claude prompt? Just so I know how to approach reviewing it. |
|
Stage 1 Stage 2
Stage 3 Stage 4
Stage 5 Background I'd add that I like xilem/masonry project. I've been reading the docs and playing with it. I like the philosophy behind it, and I think it has been designed very well. I wanted to contribute. I hope the PR meets your standards, which I find to be high. If you want me to fix something, tell me about. If you want me to close the PR, no problem. No hard feelings. My opinions about agent assisted coding. You have to put bumpers on the agent. You have to iterate, refactor, walk-through. If you follow a tight regime, you can produce a pretty good product. If you feel otherwise about the result, or the process, I know you're busy, but let me know. |
| let Ok(line) = std::str::from_utf8(buf) else { | ||
| return Ok(buf.len()); | ||
| }; |
There was a problem hiding this comment.
Alright, here's a fun one: explain this paragraph to me, without asking Claude first. Please include "This is my own understanding, I didn't ask Claude" in your answer (I'll take your word for it).
There was a problem hiding this comment.
give me either the buffer content or the length
There was a problem hiding this comment.
*why to do it this way is to bail out gracefully if i don't have valid UTF-8
There was a problem hiding this comment.
give me either the buffer content or the length
I don't think this is an accurate description of the chunk I quoted.
*why to do it this way is to bail out gracefully if i don't have valid UTF-8
I dispute the idea that it bails gracefully. At the very least, it drops data silently.
There was a problem hiding this comment.
I didn't say it bails out gracefully. I said "to bail out," as in the caller would use it to bail out. Your objection about silent data loss is still valid, but doesn't accurately contradict what I said. I can provide an example if it would serve to clarify my meaning.
Both parts of my answer are succinct, but accurate.
The buffer content itself — Ok(line) binds the decoded &str.
The length of the buffer — Ok(buf.len()) returns how many bytes were consumed.
Your point about data loss is probably more substantive, though.
I'll stop here and give you the last word, if that's what you want.
Replaces the always-on debug-mode file log with a per-frame in-memory buffer that captures tracing events emitted during
run_rewrite_passesand dumps them to a$TMPDIR/masonry-*-panic.logfile only when the process panics. Closes #1556.