-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
enable no-copy stack switching implementation #29578
Conversation
c7083f0
to
4e560e0
Compare
32bit failure on both Win and Linux, looks like we are running out of (virtual) memory? |
Ah, of course the limits will have to be much lower on 32-bit. |
de2798a
to
81e9b34
Compare
Falls back to stack copying if there are too many stack mappings. Also adds some code to free unused pooled stacks.
81e9b34
to
8a5866a
Compare
🎉 |
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.
Yes, this does look good to me.
# else | ||
# define MAX_STACK_MAPPINGS 500 | ||
# endif | ||
#endif |
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.
we should probably make these the default values for a runtime variable, since several of these are dependent on the amount of memory available in the system (currently these are set for the size of our CI machines, but user machines may be much larger or smaller), so it may make sense for some users to need to configure these
@JeffBezanson: Is there some formal way to propose a change for inclusion into the 1.1 release? This change will fix a long-standing issue in Gtk.jl (JuliaGraphics/Gtk.jl#161) but Gtk.jl can just move on (and remove the sigatom workaround) once this is in an official Julia release. |
It is on master so it's included in 1.1. |
Everything on master goes into the next minor release; backports go into patch releases. |
Ah great, I was a little confused by the patch releases but it makes of course much sense that way. |
Falls back to stack copying if there are too many stack mappings. Confirmed the
read
test passes with this. Hopefully this makes the new implementation robust enough to use by default. Also reduced the default stack size from 8MB to 4MB; 8MB is rather large.