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

Thoroughly nospecialize all functions; add no-alloc, no-specialize test. #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NHDaly
Copy link
Member

@NHDaly NHDaly commented Jun 6, 2023

Make sure that all functions in ExceptionUnwrapping are marked nospecialize. We don't want to pay the wasted compilation time at runtime, since these are all going to be called in exceptional cases, certainly not in a hot loop, and because we've seen crashes caused by a stackoverflow in type inference while attempting to specialize the code to handle a StackOverflowException! 😅

This time, we add a unit test to ensure that that these functions do not allocate and do not incur new compilation when called with novel arguments.

This is a follow-up to #6 and #15.

NHDaly added a commit that referenced this pull request Jun 6, 2023
Unclear why this was left out from
#6...

There's still probably more improvement to be made here, but I ran into
test failures on julia master, so we're leaving that as a separate
change in #14.
NHDaly added a commit that referenced this pull request Jun 6, 2023
Unclear why this was left out from
#6...

There's still probably more improvement to be made here, but I ran into
test failures on julia master, so we're leaving that as a separate
change in #14.
Make sure that _all_ functions in ExceptionUnwrapping are marked
nospecialize. We don't want to pay the wasted compilation time at
runtime, since these are all going to be called in _exceptional_ cases,
certainly not in a hot loop, and because we've seen crashes caused by a
stackoverflow in type inference while attempting to specialize the code
to handle a StackOverflowException! 😅

This time, we add a unit test to ensure that that these functions do not
allocate and do not incur new compilation when called with novel
arguments.
Comment on lines +116 to +118
@show exc
@show is_wrapped_exception(exc)
global EXC = exc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a leftover from debugging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants