refactor: move HTMLRewriter to c++ bindings #4193
Conversation
|
This looks awesome 🙂 |
|
Haven’t read code yet but exciting benchmark results, especially considering the faster one is the debug build of bun. |
| name: "AttributeIterator", | ||
| construct: true, | ||
| finalize: true, | ||
| JSType: "0b11101110", |
There was a problem hiding this comment.
There's a JSC function for creating a {next, value} object but we don't have a binding for it right now. This current implementation will be pretty slow due to not re-using the underlying JSC::Structure.
We should add a binding to this function: https://github.com/oven-sh/WebKit/blob/main/Source/JavaScriptCore/runtime/IteratorOperations.cpp#L147
|
|
||
| fn contentHandler(this: *DocEnd, comptime Callback: (fn (*LOLHTML.DocEnd, []const u8, bool) LOLHTML.Error!void), thisObject: js.JSObjectRef, globalObject: *JSGlobalObject, content: ZigString, contentOptions: ?ContentOptions) JSValue { | ||
| fn contentHandler(this: *DocEnd, comptime Callback: (fn (*LOLHTML.DocEnd, []const u8, bool) LOLHTML.Error!void), callFrame: *JSC.CallFrame, globalObject: *JSGlobalObject, content: ZigString, contentOptions: ?ContentOptions) JSValue { | ||
| if (this.doc_end == null) |
There was a problem hiding this comment.
one thing to be aware of is that callFrame is a pointer to a stack frame and things can get moved around as the stack frame changes a little
So we should prefer to reference all variables from the call frame at the beginning of the callframe, and not pass it around to non-inline functions
In this case, it's super cheap to get the this value. It's a pointer lookup without other side effects
|
This is a clear improvement over the current state, removes longstanding technical debt and improves performance. There are a couple small things to address but those can be addressed in a follow-up PR. Thank you. Merged. |
What does this PR do?
How did you verify your code works?
Using this real-wordish HTMLRewriter on ~59k records.
Here are the bench results:
make codegento regenerate the C++ and Zig code