-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: improve debug_traceBlock performance #11979
perf: improve debug_traceBlock performance #11979
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, left some suggestions
crates/rpc/rpc/src/debug.rs
Outdated
let call_config = tracer_config | ||
.clone() | ||
.into_call_config() | ||
.map_err(|_| EthApiError::InvalidTracerConfig)?; | ||
|
||
let mut inspector = TracingInspector::new( | ||
let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also slightly redundant work if the inspector is present, we have this a few time and we can definitely solve this more elegantly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah turns out we can't easily do this because we need the config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ca302df
to
ebbc8e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, ty
crates/rpc/rpc/src/debug.rs
Outdated
let call_config = tracer_config | ||
.clone() | ||
.into_call_config() | ||
.map_err(|_| EthApiError::InvalidTracerConfig)?; | ||
|
||
let mut inspector = TracingInspector::new( | ||
let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah turns out we can't easily do this because we need the config
Fixes #11066
Instead of creating TracingInspector on each call just use the optionally passed inspector and instead of cloning the whole
GethDebugTracingOptions
just clone the parts of it when necessary.One caveat of this approach is that if inspector was initialized with
CallConfig
and passed forPreStateTracer
then this would not work, so caller of thetrace_transaction
needs to make sure that correct inspector is passed for the tracing.