Created on 2013-11-07 01:02 by haypo, last changed 2013-11-07 20:51 by python-dev. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| patch_19514.diff | fhahn, 2013-11-07 15:51 | review | ||
| merge_py_identifiers.patch | andrei.duma, 2013-11-07 15:54 | review | ||
| merge_py_identifiers_sorted.patch | andrei.duma, 2013-11-07 16:01 | review | ||
| Messages (13) | |||
|---|---|---|---|
| msg202300 - (view) | Author: STINNER Victor (haypo) *  |
Date: 2013-11-07 01:02 | |
Some C files use more than once the same _Py_IDENTIFIER identifier. It would be more efficient to merge duplicated identifiers. Just move the definition to the top of the file. _Py_IDENTIFIER(as_integer_ratio): Modules/_datetimemodule.c:1569 _Py_IDENTIFIER(as_integer_ratio): Modules/_datetimemodule.c:1668 _Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1282 _Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1312 _Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1342 _Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4210 _Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4249 _Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4812 _Py_IDENTIFIER(__len__): Objects/typeobject.c:5071 _Py_IDENTIFIER(__len__): Objects/typeobject.c:5235 _Py_IDENTIFIER(insert): Modules/_bisectmodule.c:198 _Py_IDENTIFIER(insert): Modules/_bisectmodule.c:93 _Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:2638 _Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:3596 _Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:4532 _Py_IDENTIFIER(strftime): Modules/_datetimemodule.c:1280 _Py_IDENTIFIER(strftime): Modules/_datetimemodule.c:2679 |
|||
| msg202302 - (view) | Author: STINNER Victor (haypo) * |
Date: 2013-11-07 01:08 | |
See also issue #19515 which is more general. |
|||
| msg202308 - (view) | Author: STINNER Victor (haypo) * |
Date: 2013-11-07 07:47 | |
Oh, I forgot: _Py_IDENTIFIER(__delitem__): Objects/typeobject.c:5132 _Py_IDENTIFIER(__delitem__): Objects/typeobject.c:5183 |
|||
| msg202310 - (view) | Author: Andrei Dorian Duma (andrei.duma) * | Date: 2013-11-07 08:05 | |
I'll provide a patch later tonight. |
|||
| msg202347 - (view) | Author: (fhahn) | Date: 2013-11-07 15:51 | |
I've merged the _Py_IDENTIFIER identifiers mentioned above. I stumbled over anohter instance where _Py_IDENTIFIER is used more than once: _Py_IDENTIFIER(__setitem__) : Objects/typeobject.c#l5133 _Py_IDENTIFIER(__setitem__) : Objects/typeobject.c#l5184 |
|||
| msg202349 - (view) | Author: Martin v. Löwis (loewis) * |
Date: 2013-11-07 15:52 | |
As a matter of style, I suggest that all identifiers are moved to the top of a file if some of them live there. IOW, it's (IMO) unstylish to have some at the top, and some in the middle (although this works perfectly fine, of course). |
|||
| msg202350 - (view) | Author: Martin v. Löwis (loewis) * |
Date: 2013-11-07 15:53 | |
Another matter of style: I suggest alphabetical order for the identifiers, at least when the list gets long. |
|||
| msg202352 - (view) | Author: Andrei Dorian Duma (andrei.duma) * | Date: 2013-11-07 15:54 | |
The patch I promised above. |
|||
| msg202356 - (view) | Author: Andrei Dorian Duma (andrei.duma) * | Date: 2013-11-07 16:01 | |
I added a new patch with sorted _Py_IDENTIFIERs. Regarding all identifiers at the top, I guess it might be more stylish, but it might affect performance. I'm not sure, though. |
|||
| msg202358 - (view) | Author: STINNER Victor (haypo) * |
Date: 2013-11-07 16:05 | |
merge_py_identifiers_sorted.patch looks good to me. |
|||
| msg202375 - (view) | Author: Roundup Robot (python-dev) | Date: 2013-11-07 17:47 | |
New changeset 4a09cc62419b by Martin v. Löwis in branch 'default': Issue #19514: Deduplicate some _Py_IDENTIFIER declarations. http://hg.python.org/cpython/rev/4a09cc62419b |
|||
| msg202376 - (view) | Author: Martin v. Löwis (loewis) * |
Date: 2013-11-07 17:48 | |
Thanks for the patch. Note: moving all identifiers would not have made a difference. They are static variables, so from a run-time point of view, there is no difference whether they are inside or outside of functions. |
|||
| msg202380 - (view) | Author: Roundup Robot (python-dev) | Date: 2013-11-07 20:51 | |
New changeset cb4c964800af by Victor Stinner in branch 'default': Issue #19514: Add Andrei Dorian Duma to Misc/ACKS for changeset 4a09cc62419b http://hg.python.org/cpython/rev/cb4c964800af |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2013-11-07 20:51:13 | python-dev | set | messages: + msg202380 |
| 2013-11-07 17:48:57 | loewis | set | status: open -> closed resolution: fixed messages: + msg202376 |
| 2013-11-07 17:47:09 | python-dev | set | nosy:
+ python-dev messages: + msg202375 |
| 2013-11-07 16:05:39 | haypo | set | messages: + msg202358 |
| 2013-11-07 16:01:58 | andrei.duma | set | files:
+ merge_py_identifiers_sorted.patch messages: + msg202356 |
| 2013-11-07 15:54:34 | andrei.duma | set | files:
+ merge_py_identifiers.patch messages: + msg202352 |
| 2013-11-07 15:53:22 | loewis | set | messages: + msg202350 |
| 2013-11-07 15:52:42 | loewis | set | nosy:
+ loewis messages: + msg202349 |
| 2013-11-07 15:51:25 | fhahn | set | files:
+ patch_19514.diff nosy: + fhahn messages: + msg202347 keywords: + patch |
| 2013-11-07 12:34:26 | icordasc | set | nosy:
+ icordasc |
| 2013-11-07 08:13:53 | seydou | set | nosy:
+ seydou |
| 2013-11-07 08:05:50 | andrei.duma | set | messages: + msg202310 |
| 2013-11-07 07:47:43 | haypo | set | messages: + msg202308 |
| 2013-11-07 05:35:32 | andrei.duma | set | nosy:
+ andrei.duma |
| 2013-11-07 01:08:36 | haypo | set | messages: + msg202302 |
| 2013-11-07 01:02:53 | haypo | create | |