Set stdout and stdin to binary mode on Python 2 and Windows #1150
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1150 +/- ##
==========================================
- Coverage 92.78% 92.73% -0.06%
==========================================
Files 57 57
Lines 6902 6906 +4
==========================================
Hits 6404 6404
- Misses 498 502 +4
Continue to review full report at Codecov.
|
| file.flush() | ||
| # Python on Windows don't throw EPIPE errors for pipes. So reraise them with | ||
| # the correct type and error number. | ||
| except OSError: |
There was a problem hiding this comment.
Can we maybe check if the OS is windows here and otherwise just raise the error again? I generally don't like it when exceptions are caught that might be happening, because of an unrelated issue.
| elif sys.platform == 'win32': | ||
| import msvcrt | ||
| msvcrt.setmode(stdout.fileno(), os.O_BINARY) | ||
| msvcrt.setmode(stdin.fileno(), os.O_BINARY) |
There was a problem hiding this comment.
That makes a LOT of sense. Thanks for finding it. How did you realize that was the solution?
There was a problem hiding this comment.
The comment about control characters being consumed on Windows made me think that something were wrong with how the streams were open. Adding the -u option to the Python command confirmed my suspicion as this option forces the streams to be open in binary mode. However, it also make the streams unbuffered which has a big impact on performance (as seen in PR #1149) so I searched another way to put the streams in binary mode and found this solution.
|
Awesome, thanks! |
[READY] Include Jedi performance improvements This includes PRs davidhalter/jedi#1149, davidhalter/jedi#1150, davidhalter/jedi#1151, and davidhalter/jedi#1152 which significantly improve performance especially on Windows and Python 2. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/1056) <!-- Reviewable:end -->
In PR #1046, a workaround was implemented on Windows to avoid control characters on stdin and stdout because those streams are open in text mode on Python 2. The workaround consists to convert the binary data to its hexadecimal representation. The issue with that approach is that it can be really inefficient when the data is big. A better solution is to put stdin and stdout in binary mode on Windows and Python 2.
Here are some measurements obtained with this script when completing the
os,numpy, andcv2(opencv-python) modules before and after the changes on Windows:As you can see, speed improvements are huge for the
cv2module on Python 2 (up to 20 times faster). Only completing the os module on Python 2 is slower but the speed improvement in PR #1149 takes care of that.This change is