Skip to content

remove *_INTERNED opcodes from marshal #75558

Open
Open
remove *_INTERNED opcodes from marshal#75558
Labels
3.7 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

Description

@benjaminp
benjaminp
opened on Sep 7, 2017
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']

Activity

  1. benjaminp commented on Sep 7, 2017

    @benjaminp
    benjaminp
    on Sep 7, 2017
    ContributorAuthor

    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

  2. benjaminp
    added
    3.7 (EOL)end of life
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    type-featureA feature request or enhancement
    on Sep 7, 2017

  3. serhiy-storchaka commented on Sep 7, 2017

    @serhiy-storchaka
    serhiy-storchaka
    on Sep 7, 2017
    Member

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

  4. benjaminp commented on Sep 7, 2017

    @benjaminp
    benjaminp
    on Sep 7, 2017
    ContributorAuthor

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

  5. methane commented on Sep 7, 2017

    @methane
    methane
    on Sep 7, 2017
    Member

    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".

  6. benjaminp commented on Sep 7, 2017

    @benjaminp
    benjaminp
    on Sep 7, 2017
    ContributorAuthor

    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.

  7. methane commented on Sep 7, 2017

    @methane
    methane
    on Sep 7, 2017
    Member

    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.

  8. benjaminp commented on Sep 7, 2017

    @benjaminp
    benjaminp
    on Sep 7, 2017
    ContributorAuthor

    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.

  9. methane commented on Jul 11, 2018

    @methane
    methane
    on Jul 11, 2018
    Member

    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.

  10. ezio-melotti
    transferred this issue fromon Apr 10, 2022

  11. gpshead commented on Oct 4, 2026

    @gpshead
    gpshead
    on Oct 4, 2026
    Member

    Interning does cause reproducibility problems - #129724

  12. methane commented on Oct 4, 2026

    @methane
    methane
    on Oct 4, 2026
    Member

    When I made that comment in 2018, the free-threaded build did not exist, and only Unicode strings could be interned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.7 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions