[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for helpful review Benjamin.

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Roundup Robot]

Roundup Robot added the comment:

New changeset 861ddad3e0c1 by Serhiy Storchaka in branch 'default':
Issue #25856: The __module__ attribute of extension classes and functions
https://hg.python.org/cpython/rev/861ddad3e0c1

--
nosy: +python-dev

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Benjamin Peterson]

Benjamin Peterson added the comment:

I'm not too worried about slowing down __module__ especially since it's
not any slower for heap types or types in builtins.

On Thu, Sep 8, 2016, at 14:39, Serhiy Storchaka wrote:
> 
> Serhiy Storchaka added the comment:
> 
> This is what my first path does. But this slows down retrieving the
> __module__ attribute (from 0.2 usec to 0.4 usec on my computer). Maybe I
> haven't bother.
> 
> Interning __name__ and __qualname__ is less important, because different
> functions and classes usually have different names.
> 
> --
> 
> ___
> Python tracker 
> 
> ___

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

This is what my first path does. But this slows down retrieving the __module__ 
attribute (from 0.2 usec to 0.4 usec on my computer). Maybe I haven't bother.

Interning __name__ and __qualname__ is less important, because different 
functions and classes usually have different names.

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Benjamin Peterson]

Benjamin Peterson added the comment:

I think it's better not write into the type dict in a getter. You might just 
use PyUnicode_InternFromString every time.

FWIW, __name__ also has this behavior.

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It can be NULL in very rare cases. See for example issue26906.

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Benjamin Peterson]

Benjamin Peterson added the comment:

I don't think it can be NULL either.

On Wed, Sep 7, 2016, at 11:36, Serhiy Storchaka wrote:
> 
> Serhiy Storchaka added the comment:
> 
> Indeed, the PyDict_Check() check can be omitted.
> 
> --
> 
> ___
> Python tracker 
> 
> ___

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Indeed, the PyDict_Check() check can be omitted.

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Benjamin Peterson]

Benjamin Peterson added the comment:

I don't understand why you need to check the validity of tp_dict at all. We 
generally assume it's a dict.

--

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Good catch Benjamin! There is yet one bug -- the type type already has the 
__module__ attribute, but it is a descriptor, not a string.

Updated patch fixes these bugs.

--
Added file: http://bugs.python.org/file5/intern_and_cache___module__3.patch

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Benjamin Peterson]

Benjamin Peterson added the comment:

I don't understand this code:
   type->tp_dict && PyDict_Check(type->tp_dict)
since the code explicitly assume it's not NULL and access it as a dict earlier 
in the function

--
nosy: +benjamin.peterson

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Added file: http://bugs.python.org/file44431/intern_and_cache___module__2.patch

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Could anyone please make a review?

--
assignee:  -> serhiy.storchaka

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Removed file: http://bugs.python.org/file41443/intern_and_cache___module__.patch

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Added file: http://bugs.python.org/file41444/intern_and_cache___module__.patch

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Interning the __module__ string causes small performance hit:

$ ./python -m timeit -s "from itertools import chain" -- "chain.__module__; 
chain.__module__; chain.__module__; chain.__module__; chain.__module__; 
chain.__module__; chain.__module__; chain.__module__; chain.__module__; 
chain.__module__"

Unpatched:  1.93 usec per loop
Patched:4.09 usec per loop

This can be avoided if cache created string in type's __dict__. Following patch 
makes __module__ retrieving for non-heap types as fast as for heap types:

Patched2:   0.871 usec per loop

--
type: performance -> enhancement
Added file: http://bugs.python.org/file41443/intern_and_cache___module__.patch

___
Python tracker 

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

[issue25856] The __module__ attribute of non-heap classes is not interned

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Actually this is not specific to itertools. Every non-heap class not from the 
builtins module is affected.

Proposed patch just interns the __module__ value. The patch also cleans up the 
code.

--
keywords: +patch
stage:  -> patch review
title: The __module__ attribute of itertools members is not interned -> The 
__module__ attribute of non-heap classes is not interned
Added file: http://bugs.python.org/file41356/intern___module__.patch

___
Python tracker 

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