Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't enable GC for classes that don't add new fields #67914

Closed
eltoder mannequin opened this issue Mar 20, 2015 · 11 comments
Closed

Don't enable GC for classes that don't add new fields #67914

eltoder mannequin opened this issue Mar 20, 2015 · 11 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@eltoder
Copy link
Mannequin

eltoder mannequin commented Mar 20, 2015

BPO 23726
Nosy @pitrou, @benjaminp, @njsmith, @eltoder, @serhiy-storchaka
Files
  • class_gc.diff
  • class_gc2.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2015-04-13.18:10:31.603>
created_at = <Date 2015-03-20.20:25:57.755>
labels = ['interpreter-core', 'performance']
title = "Don't enable GC for classes that don't add new fields"
updated_at = <Date 2015-04-13.19:30:52.528>
user = 'https://github.com/eltoder'

    bugs.python.org fields:

    activity = <Date 2015-04-13.19:30:52.528>
actor = 'eltoder'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-13.18:10:31.603>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2015-03-20.20:25:57.755>
creator = 'eltoder'
dependencies = []
files = ['38610', '38624']
hgrepos = []
issue_num = 23726
keywords = ['patch']
message_count = 11.0
messages = ['238718', '238721', '238727', '238730', '238732', '238733', '240690', '240691', '240699', '240710', '240711']
nosy_count = 6.0
nosy_names = ['pitrou', 'benjamin.peterson', 'njs', 'python-dev', 'eltoder', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue23726'
versions = ['Python 3.5']
    @eltoder
    Copy link
    Mannequin Author

    eltoder mannequin commented Mar 20, 2015

    As far as I can tell, if a new class does not add any new fields, and its base class doesn't use GC, there's no reason to enable GC for the new class.
    This is useful for creating lightweight wrappers around classes implemented in C.

    @eltoder eltoder mannequin added the performance Performance or resource usage label Mar 20, 2015
    @serhiy-storchaka serhiy-storchaka added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 20, 2015
    @serhiy-storchaka
    Copy link
    Member 

    >>> class UserIntSlots(int):
...     __slots__ = ()
...     def __repr__(s): return '5'
... 
>>> (4).__class__ = UserIntSlots
>>> 2+2
5
>>> type(2+2)
<class '__main__.UserIntSlots'>

    It looks weird.

    @eltoder
    Copy link
    Mannequin Author

    eltoder mannequin commented Mar 20, 2015

    Agreed, but this is not new. This works without my change:

    >>> class Tuple(tuple):
...   __slots__ = ()
...   def __repr__(self): return 'Imma tuple!'                                                    
... 
>>> ().__class__ = Tuple
>>> ()
Imma tuple!

    @pitrou
    Copy link
    Member

    pitrou commented Mar 20, 2015

    I wasn't aware of that :-o

    @serhiy-storchaka
    Copy link
    Member

    May be we should forbid assigning the __class__ attribute of basic builtin
    types (especially internable, such as int, str, bytes, tuple, bool, NoneType).

    @eltoder
    Copy link
    Mannequin Author

    eltoder mannequin commented Mar 20, 2015

    Actually, this is rather new -- new in 3.5. The check was relaxed in bpo-22986: https://hg.python.org/cpython/rev/c0d25de5919e
    Previously only heap types allowed re-assigning __class__.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 13, 2015

    New changeset a60b7945ef87 by Antoine Pitrou in branch 'default':
    Issue bpo-23726: Don't enable GC for user subclasses of non-GC types that don't add any new fields.
    https://hg.python.org/cpython/rev/a60b7945ef87

    @pitrou
    Copy link
    Member

    pitrou commented Apr 13, 2015

    I've pushed the patch, thank you!

    @pitrou pitrou closed this as completed Apr 13, 2015
    @eltoder
    Copy link
    Mannequin Author

    eltoder mannequin commented Apr 13, 2015

    Thank you!

    Benjamin, Nathaniel, any opinion if we should restrict reassigning __class__ for types like tuple, int and str, where some/many instances are cached?

    @njsmith
    Copy link
    Contributor

    njsmith commented Apr 13, 2015

    Yes, it probably would be a good idea to disallow assigning __class__ for immutable types.

    @eltoder
    Copy link
    Mannequin Author

    eltoder mannequin commented Apr 13, 2015

    Agreed. There's a small problem with that, as far as I know. Nothing on type declares that it is immutable.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @njsmith @pitrou @serhiy-storchaka