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.

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

Generate frozenset constants when explicitly appropriate #90551

Closed
terryjreedy opened this issue Jan 16, 2022 · 11 comments
Closed

Generate frozenset constants when explicitly appropriate #90551

terryjreedy opened this issue Jan 16, 2022 · 11 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 46393
Nosy @terryjreedy, @stevendaprano, @serhiy-storchaka, @sweeneyde

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 = None
created_at = <Date 2022-01-16.03:11:25.506>
labels = ['interpreter-core', 'type-feature', '3.11']
title = 'Generate frozenset constants when explicitly appropriate'
updated_at = <Date 2022-01-17.11:03:45.344>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2022-01-17.11:03:45.344>
actor = 'mark.dickinson'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2022-01-16.03:11:25.506>
creator = 'terry.reedy'
dependencies = []
files = []
hgrepos = []
issue_num = 46393
keywords = []
message_count = 10.0
messages = ['410666', '410668', '410673', '410674', '410683', '410684', '410688', '410692', '410706', '410751']
nosy_count = 4.0
nosy_names = ['terry.reedy', 'steven.daprano', 'serhiy.storchaka', 'Dennis Sweeney']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46393'
versions = ['Python 3.11']

@terryjreedy
Copy link
Member Author

terryjreedy commented Jan 16, 2022

The CPython compiler is capable of making frozenset constants without being explicitly asked to. Exactly how it does so is, of course, 'hidden' from python code. With current main:

>>> dis('{1,2,3}')
  1           0 BUILD_SET                0
              2 LOAD_CONST               0 (frozenset({1, 2, 3}))
              4 SET_UPDATE               1
              6 RETURN_VALUE

Suppose one wants actually wants a frozenset, not a mutable set. 'frozenset({1,2,3})' is compiled as the above followed by a frozenset call -- making an unneeded double conversion to get what already exists.
To avoid the intermediate set, one can use a constant tuple instead.

>>> dis('frozenset((1,2,3))')
  1           0 LOAD_NAME                0 (frozenset)
              2 LOAD_CONST               0 ((1, 2, 3))
              4 CALL_FUNCTION            1
              6 RETURN_VALUE

Even nicer would be

>>> dis('frozenset((1,2,3))')
  1           0 LOAD_CONST              0 (frozenset({1, 2, 3}))
              2 RETURN_VALUE

'set((1,2,3))' is compiled the same as 'frozenset((1,2,3)), but frozenset does not have the option of using a more efficient display form. I cannot think of any reason to not call frozenset during compile time when the iterable is a constant tuple.

Serhiy, I not sure how this relates to your bpo-33318 and the discussion therein about stages, but it does relate to your interest in compile time constants.

@terryjreedy terryjreedy added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Jan 16, 2022
@stevendaprano
Copy link
Member

The difficulty is that frozenset may have been shadowed, or builtins monkey-patched, so we cannot know what frozenset({1, 2, 3}) will return until runtime.

Should we re-consider a frozenset display literal?

f{1, 2, 3}

works for me.

@terryjreedy
Copy link
Member Author

Sigh. You are right. I will close this tomorrow.

This also means that 'set()' is not guaranteed to return an empty built-in set. I did think of this workaround for that:
>>> (empty:={None}).clear()
>>> empty
set()
 
Go ahead and propose something on python-ideas if you want, pointing out that only displays (and comprehensions) are guaranteed to result in a builtin.

@sweeneyde
Copy link
Member

There's also the hacky expression {*()} to get an empty set

@stevendaprano
Copy link
Member

@mdickinson
Copy link
Member

[Terry]

To avoid the intermediate set, [...]

It's not quite as bad as that: there _is_ no intermediate set (or if you prefer, the intermediate set is the same object as the final set), since the frozenset call returns its argument unchanged if it's already of exact type frozenset:

>>> x = frozenset({1, 2, 3})
>>> y = frozenset(x)
>>> y is x
True

Relevant source:

cpython/Objects/setobject.c

Lines 999 to 1003 in 09087b8

if (iterable != NULL && PyFrozenSet_CheckExact(iterable)) {
/* frozenset(f) is idempotent */
Py_INCREF(iterable);
return iterable;
}

@stevendaprano
Copy link
Member

That's not always the case though.

>>> def f():
...     return frozenset({1, 2, 3})
... 
>>> a = f.__code__.co_consts[1]
>>> a
frozenset({1, 2, 3})
>>> b = f()
>>> assert a == b
>>> a is b
False

Also see the disassembly I posted on Python-Ideas.

@mdickinson
Copy link
Member

That's not always the case though.

Sorry, yes - I see. We're not creating a frozenset from a frozenset - we're creating a frozenset from a regular set from a frozenset. :-(

Sorry for the noise.

@terryjreedy
Copy link
Member Author

Rejected by the reality of Python's dynamism, which I overall appreciate ;-).

@serhiy-storchaka
Copy link
Member

As Steven have noted the compiler-time optimization is not applicable here because name frozenset is resolved at run-time.

In these cases where a set of constants can be replaced with a frozenset of constants (in "x in {1,2,3}" and in "for x in {1,2,3}") the compiler does it.

And I don't think there is an issue which is worth changing the language. Creating a frozenset of constants is pretty rare, and it is even more rare in tight loops. The most common cases (which are pretty rare anyway) are already covered.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@terryjreedy
Copy link
Member Author

I cleaned up the 'code' examples and the next to last sentence, verified that 'x in {1,2,3}' indeed uses a precompiled frozen set constant, and agree with Serhiy that no change is needed.

@terryjreedy terryjreedy closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants