-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Wasm jiterpreter cleanup and bug fixes pt. 4 #79324
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsBlocks #78428
|
7bba844
to
e95bb55
Compare
// The solution is likely to put the address of the scratch buffer in a global that we provide | ||
// at module instantiation time, so each thread can malloc its own copy of the buffer | ||
// and then pass it in when instantiating instead of compiling the constant into the module | ||
// FIXME: Pre-allocate these buffers and their constant slots at the start before we |
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.
it would have to be dealocated when the worker shuts down, otherwise it would leak memory long term.
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.
Is worker shutdown a common occurrence?
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.
@lambdageek could tell you more, but I think yes.
#if HOST_BROWSER | ||
#define INTERP_ENTRY_UPDATE_HIT_COUNT(_method) \ | ||
mono_interp_record_interp_entry (_method) | ||
#else |
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.
Will this call into js for every interp entry ?
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.
Only until it gets jitted, I can move some of this tracking into C if necessary
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.
It should have some kind of fastpath like if (jiterpreter_enabled) etc. Also, shouldn't this be in interp_entry () ?
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.
Putting it in the generic entry is bad since those never get jitted, no point. Will put in a fast path, good point
…number of times. Reduces number in s.r.t from ~700 to ~90 Add C prefs controlling interp entry queue thresholds Use imported globals for pointer constants (will make threading/caching easier) Use base 36 for jiterpreter import names to save space Add an option controlling the use of wasm global constants (off by default) Adjust thresholds
e95bb55
to
6df022d
Compare
Blocks #78428