classification
Title: Default value identity regression
Type: resource usage Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Kay.Hayen, benjamin.peterson, josh.r, kayhayen, larry, ned.deily, python-dev, rhettinger, serhiy.storchaka, steven.daprano, yselivanov
Priority: normal Keywords: patch

Created on 2016-09-02 16:45 by Kay.Hayen, last changed 2017-12-19 08:50 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
intern_string_constants.patch serhiy.storchaka, 2016-09-25 10:37 review
refleak.patch yselivanov, 2016-11-09 13:47 review
Messages (15)
msg274257 - (view) Author: Kay Hayen (Kay.Hayen) Date: 2016-09-02 16:45
Consider this:

def defaultKeepsIdentity(arg = "str_value"):
    print(arg is "str_value")

defaultKeepsIdentity()

This has been outputing "True" on every Python release I have seen so far, but not so on 3.6.0a4. Normally string values come from a "co_const" and will be "is" identical if used as literals in a module, but no longer in this case.

This seems wasteful at best, needlessly increasing the number of strings in usage. 

Yours,
Kay
msg274262 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2016-09-02 17:19
Can confirm the expected behaviour (printing True) in Python 2.4 through 2.7, 3.3, Jython 2.5, and even venerable old Python 1.5 (where it prints 1).

But *not* IronPython 2.6, where it prints False.

In 3.6, the difference seems to be here:

py> f = defaultKeepsIdentity
py> f.__defaults__[0] is f.__code__.co_consts[1]
False
py> f.__defaults__[0] == f.__code__.co_consts[1]
True

This behaviour is not specified by the language. Caching and re-use of strings has always been subject to change. Nevertheless, perhaps it is time for this to be make a language feature: inside a function, any use of the same string literal should use the same object?
msg274263 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-02 17:23
This likely is an interference of issue27095 with issue26148. I consider this as a regression. This causes not only a waste of memory, but can affect a performance, since comparing non-identical strings is slower than comparing identical strings.
msg277313 - (view) Author: Kay Hayen (kayhayen) Date: 2016-09-24 08:03
Same with 3.6b1, still present.
msg277323 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-09-24 17:39
It would be nice to get this fixed.
msg277359 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-25 10:37
Proposed patch interns string constants recursively in tuples and frozensets. This fixes issue26148 and issue25981.
msg277748 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-30 07:40
New changeset 78bea78d9335 by Serhiy Storchaka in branch '3.5':
Issue #27942: String constants now interned recursively in tuples and frozensets.
https://hg.python.org/cpython/rev/78bea78d9335

New changeset 0ce63a7651b9 by Serhiy Storchaka in branch '3.6':
Issue #27942: String constants now interned recursively in tuples and frozensets.
https://hg.python.org/cpython/rev/0ce63a7651b9

New changeset 44af6bd21b94 by Serhiy Storchaka in branch 'default':
Issue #27942: String constants now interned recursively in tuples and frozensets.
https://hg.python.org/cpython/rev/44af6bd21b94

New changeset 7cea3bf44acb by Serhiy Storchaka in branch '2.7':
Issue #27942: String constants now interned recursively in tuples and frozensets.
https://hg.python.org/cpython/rev/7cea3bf44acb
msg277749 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-30 07:57
Thanks Naoki and Josh for reviews.

Backported to 2.7 and 3.5 since the patch also fixes other issues.

Note that the result of ``arg is "str_value"`` is implementation detail. Other Python implementations may not intern string constants. Even CPython interns only strings constants consisting of ASCII alphanumerical characters.
msg280395 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-09 13:47
The patch is causing refleaks:

    test_ast leaked [98, 98, 98] references, sum=294
    test_code leaked [2, 2, 2] references, sum=6

Please review the attached patch fixing that.  Please review.
msg280399 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-09 14:37
Looks like the attached patch also fixes

   test_trace leaked [12, 12, 12] references, sum=36
msg280400 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-09 14:37
Good catch! The patch LGTM, thanks Yury.

Interesting, what tests in test_ast leak? I expected that this branch is executed in rare circumstances.
msg280401 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-09 14:38
> Interesting, what tests in test_ast leak? I expected that this branch is executed in rare circumstances.

The test that tests it all :) test_stdlib_validates
msg280402 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-09 14:43
New changeset 41613bb27f80 by Yury Selivanov in branch '2.7':
Issue #27942: Fix memory leak in codeobject.c
https://hg.python.org/cpython/rev/41613bb27f80

New changeset 2c6825c9ecfd by Yury Selivanov in branch '3.5':
ssue #27942: Fix memory leak in codeobject.c
https://hg.python.org/cpython/rev/2c6825c9ecfd

New changeset b671ac7ae620 by Yury Selivanov in branch '3.6':
Merge 3.5 (issue #27942)
https://hg.python.org/cpython/rev/b671ac7ae620

New changeset c27269c0d619 by Yury Selivanov in branch 'default':
Merge 3.6 (issue #27942)
https://hg.python.org/cpython/rev/c27269c0d619
msg280403 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-09 14:44
Thanks for the review, Serhiy!
msg280404 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-09 14:53
> The test that tests it all :) test_stdlib_validates

:)
History
Date User Action Args
2017-12-19 08:50:39serhiy.storchakasetpull_requests: - pull_request1087
2017-03-31 16:36:36dstufftsetpull_requests: + pull_request1087
2016-11-09 14:53:59serhiy.storchakasetmessages: + msg280404
2016-11-09 14:44:24yselivanovsetpriority: release blocker -> normal
status: open -> closed
resolution: fixed
messages: + msg280403
2016-11-09 14:43:46python-devsetmessages: + msg280402
2016-11-09 14:38:49yselivanovsetmessages: + msg280401
2016-11-09 14:37:18serhiy.storchakasetmessages: + msg280400
2016-11-09 14:37:18yselivanovsetmessages: + msg280399
2016-11-09 13:47:20yselivanovsetstatus: closed -> open
files: + refleak.patch


nosy: + ned.deily, benjamin.peterson, yselivanov, larry
messages: + msg280395
resolution: fixed -> (no value)
priority: normal -> release blocker
2016-09-30 07:57:46serhiy.storchakasetstatus: open -> closed
versions: + Python 2.7, Python 3.5
messages: + msg277749

resolution: fixed
stage: patch review -> resolved
2016-09-30 07:40:02python-devsetnosy: + python-dev
messages: + msg277748
2016-09-30 06:12:45josh.rsetnosy: + josh.r
2016-09-29 13:12:14serhiy.storchakasetassignee: serhiy.storchaka
2016-09-25 10:44:49serhiy.storchakalinkissue26148 superseder
2016-09-25 10:37:35serhiy.storchakasetfiles: + intern_string_constants.patch
versions: + Python 3.7
messages: + msg277359

keywords: + patch
stage: patch review
2016-09-24 17:39:13rhettingersetnosy: + rhettinger
messages: + msg277323
2016-09-24 08:03:24kayhayensetnosy: + kayhayen
messages: + msg277313
2016-09-02 17:23:12serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg274263
2016-09-02 17:19:50steven.dapranosetnosy: + steven.daprano
messages: + msg274262
2016-09-02 16:45:37Kay.Hayencreate