remove *_INTERNED opcodes from marshal

[benjaminp]

BPO	31377
Nosy	@benjaminp, @methane, @serhiy-storchaka

^{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 = None
closed_at = None
created_at = <Date 2017-09-07.04:54:12.645>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'remove *_INTERNED opcodes from marshal'
updated_at = <Date 2018-07-11.07:58:22.225>
user = 'https://github.com/benjaminp'

bugs.python.org fields:

activity = <Date 2018-07-11.07:58:22.225>
actor = 'methane'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2017-09-07.04:54:12.645>
creator = 'benjamin.peterson'
dependencies = []
files = []
hgrepos = []
issue_num = 31377
keywords = []
message_count = 8.0
messages = ['301569', '301571', '301572', '301576', '301592', '301593', '301594', '321413']
nosy_count = 3.0
nosy_names = ['benjamin.peterson', 'methane', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31377'
versions = ['Python 3.7']

[benjaminp]

The *_INTERN opcodes inform the marsahl reader to intern the encoded string after deserialization. I believe for pycs this is pointless because PyCode_New ends up interning all strings that are interesting to intern. Writing this opcodes makes pycs non-deterministic because the intern state may be inconsistent in the writer. See https://bugzilla.opensuse.org/show_bug.cgi?id=1049186

[serhiy-storchaka]

Marshal is used not only in pyc files. It is used for fast data serialization, faster than pickle, json, etc.

[benjaminp]

Used but not really supported. Anyway, I doubt intern round-tripping is a particularly important.

[methane]

w_ref() depends on refcnt already.
I don't think removing *_INTERN opcode makes PYC reproducible.

cpython/Python/marshal.c

Lines 269 to 271 in 1f06a68

	/* if it has only one reference, it definitely isn't shared */
	if (Py_REFCNT(v) == 1)
	return 0;

I think "intern one string, then share it 10 times" is faster than
"share one string 10 times, then intern each of 10 references".

[benjaminp]

On Thu, Sep 7, 2017, at 01:17, INADA Naoki wrote:

INADA Naoki added the comment:

w_ref() depends on refcnt already.
I don't think removing *_INTERN opcode makes PYC reproducible.

cpython/Python/marshal.c

Lines 269 to 271 in 1f06a68

	/* if it has only one reference, it definitely isn't shared */
	if (Py_REFCNT(v) == 1)
	return 0;

I know—we're going to have to do something about that, too. In practice,
though, the interning behavior seems to be a bigger reproducibility
problem.

I think "intern one string, then share it 10 times" is faster than
"share one string 10 times, then intern each of 10 references".

We end up interning each reference individually currently.

[methane]

We end up interning each reference individually currently.

But interning interned string is much faster. It only checks flag.
Interning normal string requires dict lookup.

[benjaminp]

On Thu, Sep 7, 2017, at 09:46, INADA Naoki wrote:

INADA Naoki added the comment:

> We end up interning each reference individually currently.

But interning interned string is much faster. It only checks flag.
Interning normal string requires dict lookup.

We could makes sure the version in the internal marshal memo is interned
if appropriate.

[methane]

I doubt that interning cause reproduciblity problem.

AFAIK, all strings in code object are interned or not
interned deterministically.

https://bugzilla.opensuse.org/show_bug.cgi?id=1049186
This issue seems be caused by w_ref() based on object refcnt,
not interning.