Skip to content

Commit

Permalink
Auto merge of #2585 - micbou:remove-python-check, r=Valloric
Browse files Browse the repository at this point in the history
[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 -->
  • Loading branch information
homu committed Mar 26, 2017
2 parents 53e4de0 + f84796b commit 01570aa
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 38 deletions.
41 changes: 8 additions & 33 deletions python/ycm/paths.py
@@ -1,4 +1,4 @@
# Copyright (C) 2015 YouCompleteMe contributors.
# Copyright (C) 2015-2017 YouCompleteMe contributors.
#
# This file is part of YouCompleteMe.
#
Expand Down Expand Up @@ -51,19 +51,20 @@ def memoizer( *args, **kwargs ):

@Memoize
def PathToPythonInterpreter():
# Not calling the Python interpreter to check its version as it significantly
# impacts startup time.
from ycmd import utils

python_interpreter = vim.eval( 'g:ycm_server_python_interpreter' )

if python_interpreter:
if IsPythonVersionCorrect( python_interpreter ):
if _EndsWithPython( python_interpreter ):
return python_interpreter

raise RuntimeError( "Path in 'g:ycm_server_python_interpreter' option "
"does not point to a valid Python 2.6+ or 3.3+." )

python_interpreter = _PathToPythonUsedDuringBuild()
if IsPythonVersionCorrect( python_interpreter ):
if _EndsWithPython( python_interpreter ):
return python_interpreter

# On UNIX platforms, we use sys.executable as the Python interpreter path.
Expand All @@ -72,8 +73,7 @@ def PathToPythonInterpreter():
# interpreter path.
python_interpreter = ( WIN_PYTHON_PATH if utils.OnWindows() else
sys.executable )

if IsPythonVersionCorrect( python_interpreter ):
if _EndsWithPython( python_interpreter ):
return python_interpreter

# As a last resort, we search python in the PATH. We prefer Python 2 over 3
Expand All @@ -84,8 +84,7 @@ def PathToPythonInterpreter():
python_interpreter = utils.PathToFirstExistingExecutable( [ 'python2',
'python',
'python3' ] )

if IsPythonVersionCorrect( python_interpreter ):
if python_interpreter:
return python_interpreter

raise RuntimeError( "Cannot find Python 2.6+ or 3.3+. You can set its path "
Expand All @@ -104,34 +103,10 @@ def _PathToPythonUsedDuringBuild():
return None


def EndsWithPython( path ):
def _EndsWithPython( path ):
"""Check if given path ends with a python 2.6+ or 3.3+ name."""
return path and PYTHON_BINARY_REGEX.search( path ) is not None


def IsPythonVersionCorrect( path ):
"""Check if given path is the Python interpreter version 2.6+ or 3.3+."""
from ycmd import utils

if not EndsWithPython( path ):
return False

command = [ path,
# Disable site customize. Faster, and less likely to encounter
# issues with disconnected mounts (nfs, fuse, etc.)
'-S',
'-c',
"import sys;"
"major, minor = sys.version_info[ :2 ];"
"good_python = ( major == 2 and minor >= 6 ) "
"or ( major == 3 and minor >= 3 ) or major > 3;"
# If this looks weird, remember that:
# int( True ) == 1
# int( False ) == 0
"sys.exit( not good_python )" ]

return utils.SafePopen( command ).wait() == 0


def PathToServerScript():
return os.path.join( DIR_OF_YCMD, 'ycmd' )
9 changes: 4 additions & 5 deletions python/ycm/tests/paths_test.py
@@ -1,4 +1,4 @@
# Copyright (C) 2016 YouCompleteMe contributors
# Copyright (C) 2016-2017 YouCompleteMe contributors
#
# This file is part of YouCompleteMe.
#
Expand Down Expand Up @@ -26,15 +26,15 @@
MockVimModule()

from nose.tools import ok_
from ycm.paths import EndsWithPython
from ycm.paths import _EndsWithPython


def EndsWithPython_Good( path ):
ok_( EndsWithPython( path ) )
ok_( _EndsWithPython( path ) )


def EndsWithPython_Bad( path ):
ok_( not EndsWithPython( path ) )
ok_( not _EndsWithPython( path ) )


def EndsWithPython_Python2Paths_test():
Expand All @@ -50,7 +50,6 @@ def EndsWithPython_Python2Paths_test():
yield EndsWithPython_Good, path



def EndsWithPython_Python3Paths_test():
python_paths = [
'python3',
Expand Down

0 comments on commit 01570aa

Please sign in to comment.