Marshal: performance regression with versions 3 and 4

[vstinner]

BPO	20416
Nosy	@loewis, @pitrou, @kristjanvalur, @vstinner, @larryhastings, @serhiy-storchaka, @vajrasky
Files	marshal3_numbers.patch bench.py marshal_small_ints_refs.patch marshal_refs_by_value.patch bench_issue20416.py marshal_refs_by_value_3.patch marshal_hashtable.patch marshal_hashtable_2.patch

^{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/serhiy-storchaka'
closed_at = <Date 2015-02-11.15:53:15.720>
created_at = <Date 2014-01-28.10:10:48.757>
labels = ['interpreter-core', 'performance']
title = 'Marshal: performance regression with versions 3 and 4'
updated_at = <Date 2015-02-11.15:53:15.719>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-02-11.15:53:15.719>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-02-11.15:53:15.720>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2014-01-28.10:10:48.757>
creator = 'vstinner'
dependencies = []
files = ['33770', '33771', '34071', '37894', '37895', '38012', '38013', '38088']
hgrepos = []
issue_num = 20416
keywords = ['patch']
message_count = 25.0
messages = ['209524', '209525', '209526', '209528', '209530', '209531', '209532', '209533', '209534', '209535', '211164', '211173', '211691', '234903', '235050', '235384', '235385', '235469', '235479', '235485', '235713', '235721', '235722', '235746', '235753']
nosy_count = 8.0
nosy_names = ['loewis', 'pitrou', 'kristjan.jonsson', 'vstinner', 'larry', 'python-dev', 'serhiy.storchaka', 'vajrasky']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue20416'
versions = ['Python 3.5']

[vstinner]

Attached patched disables references for int and float types.

[vstinner]

Use attached bench.py to compare performances.

Without the patch:
---
dumps v0: 389.6 ms
data size v0: 45582.9 kB
loads v0: 573.3 ms

dumps v1: 391.4 ms
data size v1: 45582.9 kB
loads v1: 558.0 ms

dumps v2: 166.9 ms
data size v2: 41395.4 kB
loads v2: 482.2 ms

dumps v3: 431.2 ms
data size v3: 41395.4 kB
loads v3: 543.8 ms

dumps v4: 361.8 ms
data size v4: 37000.9 kB
loads v4: 560.4 ms
---

With the patch:
---
dumps v0: 391.4 ms
data size v0: 45582.9 kB
loads v0: 578.2 ms

dumps v1: 392.3 ms
data size v1: 45582.9 kB
loads v1: 556.8 ms

dumps v2: 167.7 ms
data size v2: 41395.4 kB
loads v2: 484.6 ms

dumps v3: 170.3 ms
data size v3: 41395.4 kB
loads v3: 467.0 ms

dumps v4: 122.8 ms
data size v4: 37000.9 kB
loads v4: 468.9 ms
---

dumps v3 is 60% faster, loads v3 is also 14% *faster*.

dumps v4 is 66% faster, loads v4 is 16% faster.

[vstinner]

Performance of Python 3.3.3+:
---
dumps v0: 374.8 ms
data size v0: 45582.9 kB
loads v0: 625.3 ms

dumps v1: 374.6 ms
data size v1: 45582.9 kB
loads v1: 605.1 ms

dumps v2: 152.9 ms
data size v2: 41395.4 kB
loads v2: 556.5 ms
---

So with the patch, the Python 3.4 default version (4) is *faster* (dump 20% faster, load 16% faster) and produces *smaller files* (10% smaller).

[vstinner]

Oh by the way, on Python 3.4, the file size (on version 3 and 4) is unchanged with my patch. Writing a reference produces takes exactly the same size than an integer.

[vajrasky]

I am doing clinic conversion for marshal module so I am adding myself to nosy list to make sure both tickets are synchronized.

http://bugs.python.org/issue20185

[vstinner]

I am doing clinic conversion for marshal module so I am adding myself to nosy list to make sure both tickets are synchronized.

The Derby is suspended until the release of Python 3.4 final.

I consider this issue as an important performance regression that should be fixed before Python 3.4 final.

[serhiy-storchaka]

Did you tested for numerous shared int and floats? [1000] * 1000000 and [1000.0] * 1000000? AFAIK this was important use cases for adding 3 or 4 versions.

[vstinner]

Did you tested for numerous shared int and floats? [1000] * 1000000 and [1000.0] * 1000000? AFAIK this was important use cases for adding 3 or 4 versions.

Here are new benchmarks on Python 3.4 with:

Integers: [1000] * 1000000
Floats: [1000.0] * 1000000

Integers, without the patch:

dumps v3: 62.8 ms
data size v3: 4882.8 kB
loads v3: 10.7 ms

Integers, with the patch:

dumps v3: 18.6 ms (-70%)
data size v3: 4882.8 kB (same size)
loads v3: 27.7 ms (+158%)

Floats, without the patch:

dumps v3: 62.5 ms
data size v3: 4882.8 kB
loads v3: 11.0 ms

Floats, with the patch:

dumps v3: 29.3 ms (-53%)
data size v3: 8789.1 kB (+80%)
loads v3: 25.5 ms (+132%)

The version 3 was added by:
---
changeset: 82816:01372117a5b4
user: Kristján Valur Jónsson <sweskman@gmail.com>
date: Tue Mar 19 18:02:10 2013 -0700
files: Doc/library/marshal.rst Include/marshal.h Lib/test/test_marshal.py Misc/NEWS Python/marshal.c
description:
Issue bpo-16475: Support object instancing, recursion and interned strings in marshal
---

This issue tells about "sharing string constants, common tuples, even common code objects", not sharing numbers.

For real data, here are interesting numbers:
http://bugs.python.org/issue16475#msg176013

Integers only represent 4.8% of serialized data, and only 8.2% of these integers can be shared. (Floats represent 0.29%.) Whereas strings repsent 58% and 57% can be shared.

[loewis]

For the record, format 3 was added through bpo-16475, format 4 was added through bpo-19219.

In msg175962, Kristjan argued that there is no reason _not_ to share int objects, e.g. across multiple code objects. Now it seems that this argument is flawed: there is a reason, namely the performance impact.

OTOH, I consider both use case (marshaling a large number of integers, and desiring to share ints across code objects) equally obscure: you shouldn't worry about marshal performance too much if you have loads of tiny int objects, and you shouldn't worry whether these ints get shared or not.

As a compromise, we could suppress the sharing for small int objects, since they are singletons, anyway. This would allow marshal to preserve/copy the object graph, while not impacting the use case that the original poster on python-dev presented.

[pitrou]

Integers, without the patch:

dumps v3: 62.8 ms
data size v3: 4882.8 kB
loads v3: 10.7 ms

Integers, with the patch:

dumps v3: 18.6 ms (-70%)
data size v3: 4882.8 kB (same size)
loads v3: 27.7 ms (+158%)

As I wrote on python-dev, dumps performance isn't important for the pyc
use case, but loads performance is. Therefore it appears this patch goes
into the wrong direction.

You are also ignoring the *runtime* benefit of sharing objects: smaller
memory footprint of the actual Python process.

[serhiy-storchaka]

As a compromise, we could suppress the sharing for small int objects, since they are singletons, anyway.

This is implementation detail.

But we can use more efficient way to memoizating small int objects.

I also suppose than efficient C implementation of IdentityDict will significant decrease an overhead (may be 2 times). As far as this is probably a first case for which IdentityDict give significant benefit, I'll provide an implementation for 3.5.

[serhiy-storchaka]

Here is a patch which special cases small integers. It decreases v3 and v4 dump time of bench.py by 10% without affecting load time. Of course in real cases the benefit will be much less.

[larryhastings]

This is not a release blocker. You guys can play with this for 3.5. FWIW I prefer Serhiy's approach.