Skip to content
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

make traces own proxies and bsyms #1606

Open
t-vi opened this issue Jan 6, 2025 · 2 comments
Open

make traces own proxies and bsyms #1606

t-vi opened this issue Jan 6, 2025 · 2 comments
Labels

Comments

@t-vi
Copy link
Collaborator

t-vi commented Jan 6, 2025

Currently, we do not guard against inconsistencies in traces resulting from proxies not being linked to them except through the names attribute.

This proposes to change it:

  • make the "names" set into a dict name->proxy,
  • make Proxy.owning_trace / BoundSymbol.owning_trace a weak ref to the owning trace,
  • have a mechanism to copy proxies and bsyms to a trace,
  • enforce (at least in debug mode but maybe in general?) that a proxy/bsym is used in a trace only if it is in names and the owning trace is using it.

Note that many of the hard to debug "key error" issues in 2024 came from inconsistencies that would be disallowed.

cc @apaz-cli

@t-vi t-vi added enhancement New feature or request tracing architecture labels Jan 6, 2025
@mruberry
Copy link
Collaborator

mruberry commented Jan 6, 2025

Do we need to update our tooling for trace transformations before/when we do this?

@t-vi
Copy link
Collaborator Author

t-vi commented Jan 6, 2025

Yes. It is a rather large change, but it will be a huge QOL improvement avoiding things that lead to subtle bugs today.
Note that we have consistency issues (eg. mixing "old trace symbols" and "new trace symbols" in the visitor transformation leading to inconsistent shape data) with the tooling at the moment, so the required tooling changes are probably in places we need to work on.

@t-vi t-vi closed this as completed Jan 6, 2025
@t-vi t-vi reopened this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants