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

Use cPickle on Python 2 if available #1151

Merged
merged 1 commit into from Jun 21, 2018
Merged

Conversation

micbou
Copy link
Contributor

@micbou micbou commented Jun 20, 2018

We should use the cPickle module if available since it is a C implementation of pickle that is way faster.

Here are the difference between pickle and cPickle measured with this script when completing the os, numpy, and cv2 (opencv-python) modules on Ubuntu 18.04 64-bit:

Interpreter Python 2.7 Python 3.6
Env. Python 2.7 Python 3.6 Python 2.7 Python 3.6
Module pickle cPickle pickle cPickle pickle cPickle pickle cPickle
os 0.17s 0.107s 0.18s 0.156s 0.0946s 0.0741s 0.0738s 0.0728s
numpy 0.245s 0.199s 0.261s 0.242s 0.209s 0.184s 0.181s 0.179s
cv2 0.924s 0.652s 1.05s 0.932s 0.519s 0.355s 0.471s 0.47s

We can see a significant improvement for all combinations of Python except when the interpreter running Jedi and the environment are both Python 3 (which is expected since there is no cPickle on Python 3).


This change is Reviewable

Attempt to load the C version of pickle on Python 2 as it is way faster.
@codecov-io
Copy link

Codecov Report

Merging #1151 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1151   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files          57       57           
  Lines        6902     6902           
=======================================
  Hits         6404     6404           
  Misses        498      498

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e96ebbe...fabd8d5. Read the comment docs.

@davidhalter
Copy link
Owner

I'm a bit surprised that Python3 is faster with importing cPickle. cPickle doesn't even exist there. This pull request should therefore not change anything.

@davidhalter
Copy link
Owner

Oh, I realize now that it makes sense that it's faster if you run the interpreter as Python2. Sorry my bad.

Thanks for researching this!

@davidhalter davidhalter merged commit 197aa22 into davidhalter:master Jun 21, 2018
@micbou micbou deleted the cpickle branch June 21, 2018 19:57
zzbot added a commit to ycm-core/ycmd that referenced this pull request Jun 23, 2018
[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 -->
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.

None yet

3 participants