feat: add tracing instrumentation for basic things #131
Conversation
lib/engine-universal/src/link.rs
Outdated
| allocated_functions, | ||
| jt_offsets, | ||
| function_relocations, | ||
| allocated_sections, | ||
| section_relocations, | ||
| trampolines |
There was a problem hiding this comment.
Is there a reason you didn't use skip_all here?
There was a problem hiding this comment.
Hmm I think I originally wanted to keep some info, and eventually gave up and skipped everything — that said the PR is not ready for in-depth review yet, sorry for having forgotten to open it as draft, I just published to get feedback about whether other people think tracing should be an optional or mandatory dependency 😅
There was a problem hiding this comment.
Got it. My feeling is that there are very little we can hope to include as a field in spans today, so to me it makes most sense if we approach it from the direction of adding a field when we realize a need for it, rather than the other way around.
Introducing significant perf issues by accidentally debug-outputting parts of a 4MiB module structure seems like it'd be much worse than just missing that information, especially given that in production the contract can always be copied out from the database and investigated locally anyway.
There was a problem hiding this comment.
Yeah, I think this is a reasonable first pass and enough to mark the issue as solved. If we end up finding ourselves wanting more instrumentation in the future, those cases should be handled on a case-by-case basis.
See the inline comment about the ThreadPoolBuilder::build(). r=me once resolved.
| }) | ||
| .collect::<Result<Vec<CompiledFunction>, CompileError>>()? | ||
| .into_iter() | ||
| .into_iter() // TODO: why not just collect to PrimaryMap directly? |
There was a problem hiding this comment.
IIRC there are some special requirements for ParallelIterator. I tried fixing this mess once, and gave up quickly ^^
IMO we should just make rayon an unconditional dependency soon.
…ce showing huge time in the first rayon usage
Fixes #118