-
Notifications
You must be signed in to change notification settings - Fork 570
Use ES 2015 syntax to generate more compact code for blocking functions. #1137
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
Conversation
This change deduplicates variable declaration with restoration from saved context for blocking functions. This removes a significant source of duplicated code in each blocking function intro. We also introduce a helper function $restore to eliminate some repetitive snippets of generated code.
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.
Impressive size results!
Any idea if/how this affects performance of the executed code?
Good question, I assumed it should be more or less neutral, but the GoroutineSwitching benchmark shows a significant degradation:
In real life it probably isn't so bad — our CI run time did not change much, but I'll look into that and see if we can improve it. At some future point I was also considering reimplementing goroutines using ES 2015 generator functions, which should be even more compact and hopefully performant, but that would be a much bigger change. |
After some investigation I was able to make this change more or less performance-neutral, and even push size reduction a bit further. Here are some benchmarking results:
A couple of caveats for the benchmark numbers:
|
Overall it's worth noting that the like with every synthetic benchmark, the real-world difference is likely to be much, much smaller. |
As a curious side note, gopherjs on Node 12 performs in this benchmark only two times slower than native go:
Hat tip to V8 devs for that. |
@nevkontakte thanks for your work on this and gopherjs in general. I can try to test this PR on a fairly complex project and see if it works. Unfortunately some packages I'm importing started using generics... Will try to see if I can help implement that perhaps. |
Thanks @paralin, any additional testing is always welcome, although I'm fairly confident in this PR and was going to merge it as soon as I address a couple of minor comments. If you'd like to help adding support for generics, it will be much appreciated. I've been doing some research on how it could be done in GopherJS context and will try to write that down #1013 today after work. |
I'm a fan of this. Our app compiles to 5.6MB minified-and-gzipped, so even a 3% decrease in size is going to be at least 100-some kb smaller, which is significant. |
Avoid use of the `{...context}` syntax, which appears to have a significant performance penalty when used in a tight loop. In addition, apply a few further size and performance optimizations: skip unpacking $f from the context and instead fully construct it in the function epilogue.
The idea is that we can avoid referencing each variable identifier multiple times in blocking functions intro and outro using ES2015 syntax sugar for object literals and descructuring assignments.
Consider following example:
Generated code of the
producer()
function before this change:We can see that the local variable
message
is referenced 3 times in lines 1-2 and twice on the last line.Generated code after this change:
Local variable
message
is referenced only once on the line 1 and once more on the last line, plus overall the function intro code is a lot more compact.Reference app: jQuery TodoMVC (
acf500a6c32a83d8c4582d967b09a65febf0e120
)Updates #136.