Removed RefCells for dispatcher. #220
Conversation
orium
requested review from
jongiddy,
Noah-Kennedy,
scotchmist,
kflansburg,
jasnell and
a team
as code owners
orium
force-pushed
the
html-rewriter-no-refcell-for-dispatcher
branch
from
a90a9e9 to
068d90c
Compare
src/parser/state_machine/mod.rs
Outdated
|
|
||
| fn enter_cdata(&mut self, input: &[u8]); | ||
| fn leave_cdata(&mut self, input: &[u8]); | ||
| type PassingState; |
There was a problem hiding this comment.
Looking for a better name for this.
There was a problem hiding this comment.
I know it's overused as a term, but Context?
There was a problem hiding this comment.
Yeah, I think that's good: it is generic, but the nature of this type is also generic!
There was a problem hiding this comment.
yeah, was about to write that it's better to use ParserContext. Also, drop passing and shared prefixes. Just pass ctx: ParserContext around
There was a problem hiding this comment.
didn't have a time to do a deep dive in the code, so it's easier to just ask here: why it needs to be a type parameter? is it because the state is generic over S. Can we just make StateMachine trait generic then?
There was a problem hiding this comment.
why it needs to be a type parameter? is it because the state is generic over S.
It's because we need to reference it in the trait methods. (The ones in directly in the trait and the ones added by macros on the StateMachine "sub"-trait).
Can we just make StateMachine trait generic then?
Not sure if I understand: the traits are generic over the context.
orium
force-pushed
the
html-rewriter-no-refcell-for-dispatcher
branch
from
068d90c to
ee4b343
Compare
We now explicitly pass that state around. This will reduce the number of `Mutex`es in a `Send` version of the rewriter.
orium
force-pushed
the
html-rewriter-no-refcell-for-dispatcher
branch
from
ee4b343 to
efd8398
Compare
We now explicitly pass that state around. This will reduce the number of
Mutexes in aSendversion of the rewriter.Benchmarks show a median speedup of 0.4%... so, no performance difference when all benchmarks are aggregated.