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

Call os._exit() for faster shut down in the normal case #5569

Merged
merged 4 commits into from Sep 7, 2018

Conversation

Projects
None yet
4 participants
@JukkaL
Copy link
Collaborator

commented Sep 4, 2018

This can speed up self checking by about a second. We still sometimes
call normal sys.exit() when performance isn't important or when somebody
might expect a SystemExit exception.

JukkaL added some commits Sep 4, 2018

Call os._exit() for faster shut down in the normal case
This can speed up self checking by about a second. We still sometimes
call normal `sys.exit()` when performance isn't important.
@gvanrossum

This comment has been minimized.

Copy link
Member

commented Sep 4, 2018

I worry that we might encounter additional use cases that this breaks. Wouldn't it be safer to have a command-line flag "--hard-exit" that we can turn on in our internal scripts?

@JukkaL

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2018

This can have a big performance impact in the future once we migrate to a mypyc-compiled mypy -- around 20% for non-incremental runs. The impact is sufficiently big that I'd rather have this enabled by default. If this breaks some use cases, we can deal with them once they get reported. A 20% performance gain makes it worth it, I think. And if there are rare issues, I'd lean towards providing a --clean-exit option instead of making the fast way opt-in.

mypy.api is the only supported way of invoking mypy within an application and it already preserves backward compatibility. Normally invoking mypy as a process should have no observable change in semantics, or if there is a difference, it's likely caused by a bug that we can fix (such as not closing a file within mypy).

Some plugin might theoretically be affected, but the plugin API is undocumented and any changes needed to plugins should hopefully be small. If there is a real need for some plugin to do something at process exit, we can provide additional exit hooks that are guaranteed to be run even when doing a hard exit.

@gvanrossum

This comment has been minimized.

Copy link
Member

commented Sep 5, 2018

I still think an unsafe optimization should not be on by default.

I actually have a use case in internal code (I'll point you to it on Slack).

@JukkaL

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2018

Ok, let's disable it by default. We can reconsider this in the future once we've been using the option for a while internally.

@msullivan

This comment has been minimized.

Copy link
Collaborator

commented Sep 5, 2018

Maybe a good approach would be to implement the hard exit in __main__.py, which nobody should be calling into?

@JukkaL

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2018

We can can start with hiding it behind --fast-exit (now implemented). The performance difference is pretty minor until we are using a compiled mypy anyway.

@ilevkivskyi
Copy link
Collaborator

left a comment

Looks good! I think it is totally worth it (especially behind a flag).

@JukkaL JukkaL merged commit 614090b into master Sep 7, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@msullivan msullivan deleted the hard-exit branch Sep 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.