[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-30 Thread Brett Cannon


Brett Cannon  added the comment:

I just merged Hai Shi's PR, so I'm going to close assuming that took care of 
all the instances.

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-30 Thread Brett Cannon


Brett Cannon  added the comment:


New changeset 46874c26ee1fc752e2e6930efa1d223b2351edb8 by Hai Shi in branch 
'master':
bpo-39487: Merge duplicated _Py_IDENTIFIER identifiers in C code (GH-18254)
https://github.com/python/cpython/commit/46874c26ee1fc752e2e6930efa1d223b2351edb8


--
nosy: +brett.cannon

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-30 Thread hai shi


hai shi  added the comment:

If i understand clearly, msg360993 is an solution of issue39465.

--

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread STINNER Victor


STINNER Victor  added the comment:

> That means this is a useful refactoring to help identify blockers to full 
> subinterpreter support.

I don't think that subinterpreter support should block this issue, since 
currently, _Py_IDENTIFIER() does *not* support subinterpreters.

> In the subinterpreter context: perhaps it would make sense to move *all* 
> Py_IDENTIFIER declarations to file scope?

What do you mean? _Py_IDENTIFIER() macro uses "static", so no identifier is 
visible outside the current C file. The scope may be the whole file, or the 
current function, depending where it's declared.

--

Once I discussed with Eric: _Py_IDENTIFIER() should use an "interpreter local 
storage" for identifiers values. _Py_IDENTIFIER() would only be a "key" and 
_PyUnicode_FromId() would store the value somewhere in a hash table stored in 
PyInterpreterState. Something similar to the TSS API:

* PyThread_create_key()
* PyThread_delete_key_value()
* PyThread_set_key_value()
* PyThread_get_key_value()

But per interpreter, rather than being per thread.

The key can be simply the variable address in memory. It only has to be unique 
in the process.

--

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread Nick Coghlan


Nick Coghlan  added the comment:

In the subinterpreter context: perhaps it would make sense to move *all* 
Py_IDENTIFIER declarations to file scope?

That would make it much clearer which of our extension modules actually have 
hidden state for caching purposes.

If we did that though, we'd also want to update the usage instructions in 
object.h

--

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread Nick Coghlan


Nick Coghlan  added the comment:

My apologies, my comment above was based on an outdated understanding of how 
the identifier structs get initialised (it's the usage that initialises them, 
not the declaration).

That means this is a useful refactoring to help identify blockers to full 
subinterpreter support.

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

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread Nick Coghlan


Nick Coghlan  added the comment:

We can't make this change, as it means the statics get initialised before the 
Python interpreter has been initialised, and won't be reinitialised if the 
interpreter is destroyed and recreated.

--
nosy: +ncoghlan
resolution:  -> rejected
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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread Eric Snow


Eric Snow  added the comment:

FTR:

As Martin noted in #19514, there isn't any performance difference for statics, 
whether local or global.  For static locals the compiler (at least on linux) 
generates symbols named as ".<#>" and they are treated as global.

One key difference (again, at least on linux; seen using "nm") is that symbols 
for static globals preserve identifying information, like the originating 
source file.  For static locals the filename is not preserved and, when there 
are duplicates, you do not know from which function a particular symbol comes.  
So compiled symbols for static globals are *much* easier to identify in the 
source than symbols for static locals.

Hence static locals complicate something I'm working on: tooling to identify 
global variables in our source code.  So I'm a fan of efforts to remove 
duplicates (and move static locals to the explicit global namespace).  Thank 
you! :)

--
nosy: +eric.snow

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread hai shi


Change by hai shi :


--
nosy: +vstinner

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread hai shi


Change by hai shi :


--
keywords: +patch
pull_requests: +17631
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/18254

___
Python tracker 

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



[issue39487] Merge duplicated _Py_IDENTIFIER identifiers in C code

2020-01-29 Thread hai shi


New submission from hai shi :

As stinner said in issue19514

those _Py_IDENTIFIER should be merged:

./Modules/_ctypes/_ctypes.c:1054:_Py_IDENTIFIER(_type_);
./Modules/_ctypes/_ctypes.c:1132:_Py_IDENTIFIER(_type_);
./Modules/_ctypes/_ctypes.c:1494:_Py_IDENTIFIER(_type_);
./Modules/_ctypes/_ctypes.c:2071:_Py_IDENTIFIER(_type_);

./Modules/_ctypes/_ctypes.c:1692:_Py_IDENTIFIER(_as_parameter_);
./Modules/_ctypes/_ctypes.c:1759:_Py_IDENTIFIER(_as_parameter_);
./Modules/_ctypes/_ctypes.c:1826:_Py_IDENTIFIER(_as_parameter_);
./Modules/_ctypes/_ctypes.c:2256:_Py_IDENTIFIER(_as_parameter_);

./Modules/_ctypes/_ctypes.c:2474:_Py_IDENTIFIER(_check_retval_);
./Modules/_ctypes/_ctypes.c:3280:_Py_IDENTIFIER(_check_retval_);

./Modules/_pickle.c:3560:_Py_IDENTIFIER(__name__);
./Modules/_pickle.c:3979:_Py_IDENTIFIER(__name__);

./Modules/_pickle.c:4042:_Py_IDENTIFIER(__new__);
./Modules/_pickle.c:5771:_Py_IDENTIFIER(__new__);

./Python/ceval.c:5058:_Py_IDENTIFIER(__name__);
./Python/ceval.c:5134:_Py_IDENTIFIER(__name__);

./Python/import.c:386:_Py_IDENTIFIER(__spec__);
./Python/import.c:1569:_Py_IDENTIFIER(__spec__);

./Python/import.c:1571:_Py_IDENTIFIER(__path__);
./Python/import.c:1933:_Py_IDENTIFIER(__path__);

./Python/_warnings.c:487:_Py_IDENTIFIER(__name__);
./Python/_warnings.c:821:_Py_IDENTIFIER(__name__);
./Python/_warnings.c:972:_Py_IDENTIFIER(__name__);

./Python/errors.c:1012:_Py_IDENTIFIER(__module__);
./Python/errors.c:1238:_Py_IDENTIFIER(__module__);

./Objects/bytesobject.c:546:_Py_IDENTIFIER(__bytes__);
./Objects/bytesobject.c:2488:_Py_IDENTIFIER(__bytes__);

./Objects/moduleobject.c:61:_Py_IDENTIFIER(__name__);
./Objects/moduleobject.c:488:_Py_IDENTIFIER(__name__);
./Objects/moduleobject.c:741:_Py_IDENTIFIER(__name__);

./Objects/moduleobject.c:62:_Py_IDENTIFIER(__doc__);
./Objects/moduleobject.c:461:_Py_IDENTIFIER(__doc__);

./Objects/moduleobject.c:65:_Py_IDENTIFIER(__spec__);
./Objects/moduleobject.c:744:_Py_IDENTIFIER(__spec__);

./Objects/iterobject.c:107:_Py_IDENTIFIER(iter);
./Objects/iterobject.c:247:_Py_IDENTIFIER(iter);

./Objects/rangeobject.c:760:_Py_IDENTIFIER(iter);
./Objects/rangeobject.c:918:_Py_IDENTIFIER(iter);

./Objects/descrobject.c:574:_Py_IDENTIFIER(getattr);
./Objects/descrobject.c:1243:_Py_IDENTIFIER(getattr);

./Objects/odictobject.c:899:_Py_IDENTIFIER(items);
./Objects/odictobject.c:1378:_Py_IDENTIFIER(items);
./Objects/odictobject.c:2198:_Py_IDENTIFIER(items);

./Objects/fileobject.c:35:_Py_IDENTIFIER(open);
./Objects/fileobject.c:550:_Py_IDENTIFIER(open);


./Objects/typeobject.c:312:_Py_IDENTIFIER(mro);
./Objects/typeobject.c:1893:_Py_IDENTIFIER(mro);

--
components: Interpreter Core
messages: 360966
nosy: shihai1991
priority: normal
severity: normal
status: open
title: Merge duplicated _Py_IDENTIFIER identifiers in C code
type: enhancement
versions: Python 3.9

___
Python tracker 

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