[READY] Remove Python version check #2585
Conversation
|
Thanks for the PR! Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion. python/ycm/paths.py, line 53 at r1 (raw file):
I'd prefer a comment here explaining why we don't explicitly check the version of python we find. A simple "Calling the python binary to check its version significantly impacts startup time" will suffice. Comments from Reviewable |
Starting the Python interpreter to check its version is slow.
micbou
force-pushed
the
remove-python-check
branch
from
fd0ee96
to
f84796b
Compare
|
Reviewed 1 of 1 files at r2. python/ycm/paths.py, line 53 at r1 (raw file): Previously, Valloric (Val Markovic) wrote…
Done. Comments from Reviewable |
Codecov Report
@@ Coverage Diff @@
## master #2585 +/- ##
==========================================
- Coverage 86.16% 86.11% -0.05%
==========================================
Files 19 19
Lines 1951 1945 -6
==========================================
- Hits 1681 1675 -6
Misses 270 270Continue to review full report at Codecov.
|
|
Reviewed 1 of 2 files at r1, 1 of 1 files at r2. Comments from Reviewable |
|
@homu r+ |
|
📌 Commit f84796b has been approved by |
|
⚡ Test exempted - status |
[READY] Remove Python version check Even with the `-S` option (see PR #2502), starting the Python interpreter to check its version can be slow. For instance, on my machine (in PowerShell): ```ps > Measure-Command -Expression {python -S -c "pass"} ... TotalMilliseconds : 25,4387 ``` It's also not particularly useful because, in the typical case, the Python used to start ycmd will be the one from `PYTHON_USED_DURING_BUILDING` which is necessarily a supported version. In the cases where the `g:ycm_server_python_interpreter` option is specified or a Python is found in the PATH, it's up to the user to provide a supported version. If they don't, the ycmd server will probably fail to start and the following error will be displayed to them: ``` The ycmd server SHUT DOWN (restart with ':YcmRestartServer'). Unexpected exit code 1. Use the ':YcmToggleLogs' command to check the logs. ``` They can then look at the logs to understand what went wrong. Finally, the last case is when the Python is the one used by YCM itself. In that case, if the Python version is not supported, it will fail before doing the check so it doesn't matter. As always, here are the improvements on startup time: <table> <tr> <th rowspan="2">Platform</th> <th colspan="2">First run (ms)</th> <th colspan="2">Subsequent runs (ms)</th> </tr> <tr> <td>Before</td> <td>After</td> <td>Before</td> <td>After</td> </tr> <tr> <td>Ubuntu 16.04 64-bit</td> <td>147</td> <td>134</td> <td>74</td> <td>65</td> </tr> <tr> <td>macOS 10.12</td> <td>189</td> <td>165</td> <td>127</td> <td>107</td> </tr> <tr> <td>Windows 10 64-bit</td> <td>242</td> <td>200</td> <td>147</td> <td>115</td> </tr> </table> *Results obtained by running the `prof.py` script from [this branch](https://github.com/micbou/YouCompleteMe/tree/profiling-startup) on Python 3.6.* <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2585) <!-- Reviewable:end -->
Even with the
-Soption (see PR #2502), starting the Python interpreter to check its version can be slow. For instance, on my machine (in PowerShell):It's also not particularly useful because, in the typical case, the Python used to start ycmd will be the one from
PYTHON_USED_DURING_BUILDINGwhich is necessarily a supported version. In the cases where theg:ycm_server_python_interpreteroption is specified or a Python is found in the PATH, it's up to the user to provide a supported version. If they don't, the ycmd server will probably fail to start and the following error will be displayed to them:They can then look at the logs to understand what went wrong.
Finally, the last case is when the Python is the one used by YCM itself. In that case, if the Python version is not supported, it will fail before doing the check so it doesn't matter.
As always, here are the improvements on startup time:
Results obtained by running the
prof.pyscript from this branch on Python 3.6.This change is