[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:

Thanks INADA Naoki! Thanks a simple and cool optimization!

Maybe you might want to document it at 
https://docs.python.org/dev/whatsnew/3.8.html#optimizations ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:


New changeset f7e4d3642fbb88f4e6243c952a0e223fb5df1c65 by Victor Stinner (INADA 
Naoki) in branch 'master':
bpo-34100: compile: Re-enable frozenset merging (GH-10760)
https://github.com/python/cpython/commit/f7e4d3642fbb88f4e6243c952a0e223fb5df1c65


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10011

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

Change by INADA Naoki :


--
pull_requests: +10008

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:

Well, it's up to you. I will review your PR ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

INADA Naoki  added the comment:

I agree that modifying tuple is bad idea in general case.

But in case of constants, in-place modify is the easiest approach.

Our purpose is "merging" constant.  On the other hand, "build new tuple and
replace old tuple" approach makes two same constant tuples.
If old tuple is referenced from somewhere, we fail to "merging".

So I don't think we need to follow the general principle unless it reduces code.

FYI, we have similar in-place editing for interning already.
https://github.com/python/cpython/blob/master/Objects/codeobject.c#L47-L95

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:

merge_consts_recursive(): rather than modifying the 'key' tuple, would it make 
sense to *add* the new key to the cache?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 1005c84535191a72ebb7587d8c5636a065b7ed79 by Victor Stinner in 
branch 'master':
bpo-34100: Partially revert merge_consts_recursive() (GH-10743)
https://github.com/python/cpython/commit/1005c84535191a72ebb7587d8c5636a065b7ed79


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

STINNER Victor  added the comment:

> New changeset c2e1607a51d7a17f143b5a34e8cff7c6fc58a091 by Miss Islington 
> (bot) (INADA Naoki) in branch 'master'

This change introduced a lot of memory leaks (reference leaks):
https://buildbot.python.org/all/#/builders/1/builds/422

The following change fix "make && ./python -m test -R 3:3 test_code":

diff --git a/Python/compile.c b/Python/compile.c
index acb5cfe29b..cb3e73740d 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -1277,6 +1277,7 @@ merge_consts_recursive(struct compiler *c, PyObject *o)
 else {
 u = k;
 }
+Py_DECREF(k);
 Py_INCREF(u);
 PyTuple_SET_ITEM(tuple, i, u);
 i++;
@@ -1288,6 +1289,7 @@ merge_consts_recursive(struct compiler *c, PyObject *o)
 Py_DECREF(key);
 return NULL;
 }
+Py_DECREF(PyTuple_GET_ITEM(key, 1));
 PyTuple_SET_ITEM(key, 1, new);
 }
 

But I dislike the frozenset branch of merge_consts_recursive(): modifying a 
tuple is a bad idea. For example, if you replace PyTuple_SET_ITEM() with 
PyTuple_SetItem(), you get a PyErr_BadInternalCall() because the reference 
count is greater than 1.

I don't see how to rewrote properly the code, so I reverted (removed) it to let 
someone else fix the code: PR 10743.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +9990

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

Change by INADA Naoki :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[miss-islington]

miss-islington  added the comment:


New changeset c2e1607a51d7a17f143b5a34e8cff7c6fc58a091 by Miss Islington (bot) 
(INADA Naoki) in branch 'master':
bpo-34100: Merge constants recursively (GH-8341)
https://github.com/python/cpython/commit/c2e1607a51d7a17f143b5a34e8cff7c6fc58a091


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

INADA Naoki  added the comment:

This is another memory overhead comparison.
It seems merging constants reduces 2~3% memory usage.

import sys, django, flask
sys._debugmallocstats()

### master branch

class   size   num pools   blocks in use  avail blocks
-      -   -  
0  8   1 312   194
1 16   1 135   118
2 24   2 198   138
3 32  22269082
4 40  62621448
5 48 149   1246551
6 56 254   1826028
7 64 354   22300 2
8 72 208   1158959
9 80 124617327
   10 88  592708 6
...
# bytes in allocated blocks=9,261,952


### merge-const branch

class   size   num pools   blocks in use  avail blocks
-      -   -  
0  8   1 312   194
1 16   1 135   118
2 24   1 161 7
3 32  222657   115
4 40  62620260
5 48 102850167
6 56 235   1687941
7 64 346   2175147
8 72 204   1138737
9 80 122608317
   10 88  58265612
...
# bytes in allocated blocks=8,919,824

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

Change by INADA Naoki :


--
keywords: +needs review -3.7regression
stage: needs patch -> patch review
type: enhancement -> resource usage

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

INADA Naoki  added the comment:

Counting object types in logging/__pycache__/__init__.cpython-38.pyc:

master:
[('r', 1815), (')', 467), ('Z', 339), ('s', 314), ('z', 273), ('c', 157), ('N', 
154), ('a', 24), ('F', 14), ('i', 11), ('T', 8)]

GH-8341:
[('r', 1737), (')', 375), ('Z', 339), ('s', 314), ('z', 264), ('c', 157), ('N', 
138), ('a', 24), ('F', 14), ('i', 11), ('T', 8)]

It reduced about 5% objects.

I chose logging/__init__ because it's large except tests, and it's written in 
OO-based. (Large application has many OO-based code).

--
keywords:  -patch
stage: patch review -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

Change by INADA Naoki :


--
keywords: +patch
pull_requests: +7875
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[Terry J. Reedy]

Terry J. Reedy  added the comment:

I think (space) 'performance' would be a better label, as this is strictly an 
implementation improvement, not a language change.  But we often (usually? 
sometimes?) limit performance improvements to the 'next version' so we have the 
alpha/beta/candidate releases to discover possible regressions.

I think this is worth considering just because the pattern is so odd.  But if 
this is ironed out as part of a broader and better patch, great.

--
nosy: +terry.reedy
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[Brett Cannon]

Brett Cannon  added the comment:

I also agree that there's no bug here, so I'm marking this an enhancement and 
removing the regression label.

--
nosy: +brett.cannon
type: resource usage -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34100] Same constants in tuples are not merged while compile()

[INADA Naoki]

Change by INADA Naoki :


--
title: Same integers in a tuple of constant literals are not merged -> Same 
constants in tuples are not merged while compile()

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com