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

Implement fast symbol lookup in the interpreter #5638

Closed
laurentlb opened this issue Jul 19, 2018 · 4 comments
Closed

Implement fast symbol lookup in the interpreter #5638

laurentlb opened this issue Jul 19, 2018 · 4 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) type: feature request

Comments

@laurentlb
Copy link
Contributor

We talked about speeding up the interpreter by avoiding hash maps for variable lookup. We should be able to assign statically an integer to each variable.

Unfortunately, we can't do this safely right now because of a bug in the interpreter (#5637). In some cases, the name resolution is not static. Once the bug is fixed, that should be feasible.

cc @ttsugriy

@laurentlb laurentlb added type: feature request P2 We'll consider working on this in future. (Assignee optional) team-Starlark and removed category: extensibility > skylark labels Nov 21, 2018
@meisterT
Copy link
Member

I assume @alandonovan's work makes this obsolete?

@laurentlb
Copy link
Contributor Author

I'd say it's part of Alan's work. You first need the static resolution before you can generate (decent) bytecode.

@alandonovan
Copy link
Contributor

alandonovan commented May 17, 2020

What Laurent said. The following changes are necessary first:

  • decouple thread from module (CL 309983907, in review)
  • add loadBindsGlobally option to resolver (will port go.starlark.net resolver)
  • make the prelude behave like a load statement (no shared syntax trees => recordScope always on) (CL 309105613, in progress)
  • use arrays for local variables (the actual optimization; it is trivial)

@brandjon
Copy link
Member

Alan implemented this some time ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants