-
Notifications
You must be signed in to change notification settings - Fork 357
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
Use x/tools/go/packages to significantly improve speed of extract code generation #1642
base: master
Are you sure you want to change the base?
Conversation
…new x/packages. also found minimal set of Need flags.
we are not sure why the generated code tests are failing -- they don't fail when we run locally. Also, this PR is included in #1647 so could be closed depending on your preference for how to proceed. |
The performance improvement is nice, congrats for that. But it comes at the cost of adding new dependencies, other than Go standard library, something that we have avoided to do so far for this project. The mishandling of dependency may be the cause of the CI failure, but I have not yet analyzed it in details. The improvement concerns the extract command, which is rarely executed, only at I do not want to close completely the door. We could consider incorporating this kind of improvement if there is a way to isolate the extract command in its own dependency set, with a separate |
The inclusion of a dependency outside of the standard library may be unideal, but it has no concrete impact on the interpreter. Go is designed such that packages are only included in the binary and the Moreover, the dependencies that this adds are from golang.org/x, which is still part of the Go Project and maintained by the Go Team. These are not unstable packages of questionable quality; these are reasonable extensions of the standard library developed by the same developers as the standard library itself. As you can see here, Finally, this is not a low-priority, rarely executed thing for us. As part of our three projects that use yaegi extensively (a live GUI playground, a command-line shell, and an interactive data science platform), we entirely depend on including pre-compiled GUI, shell, and data science packages in the interpreter. As we develop these projects, we frequently make changes and additions that require re-generating the extract code, and it is extremely disruptive for that process to take minutes when everything else is practically instant. We appreciate the invaluable functionality provided by yaegi, and we hope that you can prioritize providing a powerful and efficient platform over avoiding any dependencies. |
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.
Ok, let's give a try to dependencies.
This PR updates
extract
to use golang.org/x/tools/go/packages instead ofgo/importer
when possible, which results in an 11x speedup for the first time and a 45x speedup for future extraction due to caching.This is the raw timing output for running
yaegi extract
on a collection of various packages of different sizes:Old:
New: