&try / backtrack() usage too expensive due to unconditional stack unwinding

[awelzel]

The initial implementation of the QUIC parser uses two &try / backtrack() constructs for parsing the initial byte and a second one for capturing all the data. During tests, the output of perf report --no-children indicates 8.5% of the samples fall into _Unwind_GetTextRelBase which seems surprising: The pcap in question produces a single analyzer.log entry only.

Concretely, processing a a 274MB pure QUIC traffic pcap with latest Zeek master (v6.1.0-dev-487-ge22bf8ebb6, RelWithDebInfo, frame pointers enabled) and latest spicy-quic parser takes ~30seconds.

Replacing the two &try / backtrack() constructs with &parse-at=self.input() and resetting the input reduces the processing time to ~24seconds. That's 20% faster for that specific pcap (and not using try/backtrack() looks actually better).

Light grepping through Spicy makes me believe that for every &try and backtrack() execution a new Backtrace object is instantiated, which unconditionally calls backtrace() and then _Unwind_GetTextRelBase. For the try / backtrack() mechanism that is unproductive work. Flamegraphs before/after below. They look quite bit different, so wonder if the backtrack() also triggers something with the fiber execution.

Anyway, try/backtrack() should probably not come with such a huge overhead or otherwise cause a warning with discouragement and alternatives to use instead.

---

Flamegraphs

try-backtrack (before)

parse-at (after)

[rsmmr]

I think it's the exception being thrown by backtrack() that internally captures the stack at construction time. Looks like that unwinding is pretty expensive. Is this Linux? We could skip that, but we'd loose the ability to show where the backtrack happened in case the exception remains uncaught.

That said, I also wouldn't want encourage such use of try/backtrack(); it's meant as an last-resort mechanism if there's nothing better available, usually to step back across some layers of parsing (hence the exception mechanism).

[awelzel]

Is this Linux? We could skip that, but we'd loose the ability to show where the backtrack happened in case the exception remains uncaught.

Yep, Linux. For the unhandled backtrack() exception the location in the .spicy file would be sufficient, doesn't seem the whole native code stack is required for sufficient error reporting.

That said, I also wouldn't want encourage such use of try/backtrack(); it's meant as an last-resort mechanism if there's nothing better available, usually to step back across some layers of parsing (hence the exception mechanism).

Yep, not arguing. Performance can likely be fixed, but also the issue in understanding control flow in anything more complex. But then, for a beginner it's an easy to understand tool in reach also for simple cases. In the docs, it's also just before the "Changing Input" section that has the better alternatives. A warning/info box in the documentation suggesting to use the set_input() / input() / parse-at constructs might prevent some misuse, too.

The spicy-postgres parser also has a &try / backtrack() to parse into a few bytes and backtrack(), but that was also me :-)

[rsmmr]

Turns out these backtraces aren't used anywhere by default anyways, only if requested explicitly through a runtime option. I'll limit capture to debug builds.