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

Add Unladen Swallow's optimizations to Python 3's pickle. #53656

Closed
avassalotti opened this issue Jul 29, 2010 · 13 comments
Closed

Add Unladen Swallow's optimizations to Python 3's pickle. #53656

avassalotti opened this issue Jul 29, 2010 · 13 comments
Assignees
Labels
extension-modules C modules in the Modules dir performance Performance or resource usage

Comments

@avassalotti
Copy link
Member

BPO 9410
Nosy @smontanaro, @jcea, @abalkin, @pitrou, @vstinner, @avassalotti
Files
  • pickle_optimizations.diff
  • issue1694050_19001.diff
  • pickle_optimizations4.diff
  • pickle_optimizations5.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/avassalotti'
    closed_at = <Date 2010-09-09.18:33:58.334>
    created_at = <Date 2010-07-29.06:26:04.924>
    labels = ['extension-modules', 'performance']
    title = "Add Unladen Swallow's optimizations to Python 3's pickle."
    updated_at = <Date 2011-03-19.02:58:22.909>
    user = 'https://github.com/avassalotti'

    bugs.python.org fields:

    activity = <Date 2011-03-19.02:58:22.909>
    actor = 'jcea'
    assignee = 'alexandre.vassalotti'
    closed = True
    closed_date = <Date 2010-09-09.18:33:58.334>
    closer = 'pitrou'
    components = ['Extension Modules']
    creation = <Date 2010-07-29.06:26:04.924>
    creator = 'alexandre.vassalotti'
    dependencies = []
    files = ['18246', '18777', '18778', '18779']
    hgrepos = []
    issue_num = 9410
    keywords = ['patch']
    message_count = 13.0
    messages = ['111895', '111904', '111958', '111960', '112091', '112956', '115718', '115723', '115724', '115725', '115726', '115771', '115957']
    nosy_count = 7.0
    nosy_names = ['skip.montanaro', 'collinwinter', 'jcea', 'belopolsky', 'pitrou', 'vstinner', 'alexandre.vassalotti']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue9410'
    versions = ['Python 3.2']

    @avassalotti
    Copy link
    Member Author

    This is a big patch. Please review at http://codereview.appspot.com/1694050/show

    This patch adds the most interesting optimizations from Unladen Swallow to Python 3's pickle.

    The core of the patch already been reviewed by Antoine and me (http://codereview.appspot.com/33070/show). One of the last issue remaining the unbounded size of the internal buffer. This shouldn't be a big issue for most uses of pickle, since the size of a pickle is often several times smaller than the object hierarchy that created it. I still hope to fix this in a following patch.

    The patch also include additional cleanups to the Pdata structure. The changes felt natural to make along with the other changes from Unladen Swallow. IIRC, these changes yield an additional 1-3% speedup.

    @avassalotti avassalotti self-assigned this Jul 29, 2010
    @avassalotti avassalotti added extension-modules C modules in the Modules dir performance Performance or resource usage labels Jul 29, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Jul 29, 2010

    Le jeudi 29 juillet 2010 à 06:26 +0000, Alexandre Vassalotti a écrit :

    New submission from Alexandre Vassalotti <alexandre@peadrop.com>:

    This is a big patch. Please review at
    http://codereview.appspot.com/1694050/show

    This patch adds the most interesting optimizations from Unladen
    Swallow to Python 3's pickle.

    The core of the patch already been reviewed by Antoine and me
    (http://codereview.appspot.com/33070/show). One of the last issue
    remaining the unbounded size of the internal buffer. This shouldn't be
    a big issue for most uses of pickle, since the size of a pickle is
    often several times smaller than the object hierarchy that created it.
    I still hope to fix this in a following patch.

    I still think this should be fixed in this patch, especially since it
    will also change benchmark numbers.

    @abalkin
    Copy link
    Member

    abalkin commented Jul 29, 2010

    For those not familiar with Unladen Swallow, can you describe what the "most interesting optimizations" are? Maybe there is an Unladen Swallow document you can point to. Would any of these optimizations apply to python implementation?

    @vstinner
    Copy link
    Member

    I'm working on bpo-3873 to add a read buffer (fixed size, 4096 bytes) to the unpickler. It's 6 to 8 times faster with the read buffer: but this patch is mainly to avoid the overhead introduced by the new I/O library (in Python2, unpickler was faster because it doesn't need to call Python functions to read some bytes). Is this feature included in this big patch?

    @smontanaro
    Copy link
    Contributor

    Look around the issues. I'm pretty sure I worked on the
    unbounded size issue at one point.

    Skip

    @birkenfeld birkenfeld changed the title Add Unladden Swallow's optimizations to Python 3's pickle. Add Unladen Swallow's optimizations to Python 3's pickle. Jul 31, 2010
    @avassalotti
    Copy link
    Member Author

    Ah, that's right Skip. You did fixed it in Unladen Swallow's trunk. I will take a look at your solution.

    http://code.google.com/p/unladen-swallow/source/diff?spec=svn1167&r=1038&format=side&path=/trunk/Modules/cPickle.c&old_path=/trunk/Modules/cPickle.c&old=988

    @pitrou
    Copy link
    Member

    pitrou commented Sep 6, 2010

    The patch doesn't apply cleanly anymore. Furthermore, I discovered some additional issues:

    • load, dump, loads and dumps from the _pickle module were never used because they were shadowed by the same functions in pickle.py
    • once the C functions above are enabled, they produce some bugs

    I'm working on an updated patch, fixing the aforementioned bugs and adding a buffer size limit.

    @avassalotti
    Copy link
    Member Author

    Antoine, I fixed these issues in the latest patch posted on Rietveld. Also, Skip added the buffer limit in Unladen Swallow (see msg112956). We just need to merge that.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 6, 2010

    Here is a patch. Benchmark numbers:

    • dumps():

    ./python -m timeit -s "import pickle, io; d={(x, 'a'): x for x in range(10000)}" "pickle.dumps(d)"

    -> before: 100 loops, best of 3: 7.47 msec per loop
    -> after: 100 loops, best of 3: 2.45 msec per loop

    • loads():

    ./python -m timeit -s "import pickle, io; d={(x, 'a'): x for x in range(10000)}; d=pickle.dumps(d)" "pickle.loads(d)"

    -> before: 100 loops, best of 3: 12.1 msec per loop
    -> after: 100 loops, best of 3: 2.62 msec per loop

    • dump():

    ./python -m timeit -s "import pickle, io; d={(x, 'a'): x for x in range(10000)}" "pickle.dump(d, io.BytesIO())"
    -> before: 100 loops, best of 3: 13.2 msec per loop
    -> after: 100 loops, best of 3: 2.54 msec per loop

    • load():

    ./python -m timeit -s "import pickle, io; d={(x, 'a'): x for x in range(10000)}; d=pickle.dumps(d)" "pickle.load(io.BytesIO(d))"
    -> before: 100 loops, best of 3: 12.7 msec per loop
    -> after: 100 loops, best of 3: 11.6 msec per loop

    As you can see, load() doesn't really benefit from the buffering improvements. The three methods see quite massive speedups.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 6, 2010

    Gosh. My patch is based on an outdated patch :(

    @pitrou
    Copy link
    Member

    pitrou commented Sep 6, 2010

    Ok, this patch merges my changes with Alexandre's previous patch. Performance is similar as the previous posted patch.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 7, 2010

    For the record, here are the unladen swallow benchmark results against stock py3k:

    ### pickle ###
    Min: 1.644222 -> 0.812691: 2.0232x faster
    Avg: 1.652311 -> 0.814994: 2.0274x faster
    Significant (t=297.660908)
    Stddev: 0.00594 -> 0.00207: 2.8732x smaller

    ### unpickle ###
    Min: 2.802217 -> 0.751013: 3.7312x faster
    Avg: 2.807074 -> 0.752646: 3.7296x faster
    Significant (t=980.311525)
    Stddev: 0.00446 -> 0.00145: 3.0831x smaller

    ### pickle_dict ###
    Min: 0.744259 -> 0.543992: 1.3681x faster
    Avg: 0.747806 -> 0.545883: 1.3699x faster
    Significant (t=114.070170)
    Stddev: 0.00266 -> 0.00293: 1.1014x larger

    ### pickle_list ###
    Min: 2.058899 -> 1.212566: 1.6980x faster
    Avg: 2.066486 -> 1.216711: 1.6984x faster
    Significant (t=269.964154)
    Stddev: 0.00534 -> 0.00459: 1.1635x smaller

    ### unpickle_list ###
    Min: 1.458531 -> 0.313154: 4.6576x faster
    Avg: 1.464023 -> 0.319126: 4.5876x faster
    Significant (t=425.745063)
    Stddev: 0.00476 -> 0.00367: 1.2976x smaller

    @pitrou
    Copy link
    Member

    pitrou commented Sep 9, 2010

    The patch was finally committed in r84653. Thanks to everyone who participated in this.

    @pitrou pitrou closed this as completed Sep 9, 2010
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants