On 17. 02. 22 2:13, Eric Snow wrote:
Thanks for the feedback.  My responses are inline below.

-eric


On Wed, Feb 16, 2022 at 6:36 AM Petr Viktorin <encu...@gmail.com> wrote:
Thank you very much for writing this down! It's very helpful to see a
concrete proposal, and the current state of this idea.
I like the change,

That's good to hear. :)

but I think it's unfortunately more complicated than
the PEP suggests.

That would be unsurprising. :)

This proposal is CPython-specific and, effectively, describes
internal implementation details.

I think that is a naïve statement. Refcounting is
implementation-specific, but it's hardly an *internal* detail.

Sorry for any confusion.  I didn't mean to say that refcounting is an
internal detail.  Rather, I was talking about how the proposed change
in refcounting behavior doesn't affect any guaranteed/documented
behavior, hence "internal".

Perhaps I missed some documented behavior?  I was going off the following:

* 
https://docs.python.org/3.11/c-api/intro.html#objects-types-and-reference-counts
* https://docs.python.org/3.11/c-api/structures.html#c.Py_REFCNT

There is
code that targets CPython specifically, and relies on the details.

Could you elaborate?  Do you mean such code relies on specific refcount values?

The refcount has public getters and setters,

Agreed.  However, what behavior do users expect and what guarantees do
we make?  Do we indicate how to interpret the refcount value they
receive?  What are the use cases under which a user would set an
object's refcount to a specific value?  Are users setting the refcount
of objects they did not create?

That's what I hoped the PEP would tell me. Instead of simply claiming that there won't be issues, it should explain why we won't have any issues.


and you need a pretty good
grasp of the concept to write a C extension.

I would not expect this to be affected by this PEP, except in cases
where users are checking/modifying refcounts for objects they did not
create (since none of their objects will be immortal).

I think that it's safe to assume that this will break people's code,

Do you have some use case in mind, or an example?  From my perspective
I'm having a hard time seeing what this proposed change would break.

That said, Kevin Modzelewski indicated [1] that there were affected
cases for Pyston (though their change in behavior is slightly
different).

[1] 
https://mail.python.org/archives/list/python-dev@python.org/message/TPLEYDCXFQ4AMTW6F6OQFINSIFYBRFCR/

IMO, the reasoning should start from the assumption that things will break, and explain why they won't (or why the breakage is acceptable). If the PEP simply tells me upfront that things will be OK, I have a hard time trusting it.

IOW, it's clear you've thought about this a lot (especially after reading your replies here), but it's not clear from the PEP. That might be editorial nitpicking, if it wasn't for the fact that I want find any gaps in your research and reasoning, and invite everyone else to look for them as well.


[...]
Every modification of a refcount causes the corresponding cache
line to be invalidated.  This has a number of effects.

For one, the write must be propagated to other cache levels
and to main memory.  This has small effect on all Python programs.
Immortal objects would provide a slight relief in that regard.

On top of that, multi-core applications pay a price.  If two threads
are interacting with the same object (e.g. ``None``)  then they will
end up invalidating each other's caches with each incref and decref.
This is true even for otherwise immutable objects like ``True``,
``0``, and ``str`` instances.  This is also true even with
the GIL, though the impact is smaller.

This looks out of context. Python has a per-process GIL. It should it go
after the next section.

This isn't about a data race.  I'm talking about how if an object is
active in two different threads (on distinct cores) then incref/decref
in one thread will invalidate the cache (line) in the other thread.
The only impact of the GIL in this case is that the two threads aren't
running simultaneously and the cache invalidation on the idle thread
has less impact.

Perhaps I've missed something?

Ah, I see. I was confused by this:
This is also true even with the GIL, though the impact is smaller.

Smaller than what? The baseline for that comparison is a hypothetical GIL-less interpreter, which is only introduced in the next section. Perhaps say something like "Python's GIL helps avoid this effect, but doesn't eliminate it."


The proposed solution is obvious enough that two people came to the
same conclusion (and implementation, more or less) independently.

Who was it? Assuming it's not a secret :)

Me and Eddit. :)  I don't mind saying so.

In the case of per-interpreter GIL, the only realistic alternative
is to move all global objects into ``PyInterpreterState`` and add
one or more lookup functions to access them.  Then we'd have to
add some hacks to the C-API to preserve compatibility for the
may objects exposed there.  The story is much, much simpler
with immortal objects

Weren't you planning a PEP on subinterpreter GIL as well? Do you want to
submit them together?

I'd have to think about that.  The other PEP I'm writing for
per-interpreter GIL doesn't require immortal objects.  They just
simplify a number of things.  That's my motivation for writing this
PEP, in fact. :)

Please think about it.
If you removed the benefits for per-interpreter GIL, the motivation section would be reduced to is memory savings for fork/CoW. (And lots of performance improvements that are great in theory but sum up to a 4% loss.)


IMO, as it is, the PEP's motivation doesn't really stand on its own.
It's only worth it as a step towards per-interpreter GIL.

I expect Eddie would argue otherwise, but I probably wouldn't have
written this PEP if it weren't for its benefit to per-interpreter GIL.

(We might have a catch-24 situation in the way we currently handle PEPs.
That would mean we need to change the process, maybe even permanently.
IMO, this PEP will be very helpful in untangling the situation.)

Glad to help. :)

Backward Compatibility
-----------------------

This proposal is completely compatible.  It is internal-only so no API
is changing.

The approach is also compatible with extensions compiled to the stable
ABI.  Unfortunately, they will modify the refcount and invalidate all
the performance benefits of immortal objects.  However, the high bit
of the refcount will still match ``_Py_IMMORTAL_REFCNT`` so we can
still identify such objects as immortal.

So, any extension that uses the stable ABI will break an invariant.
What'll be the impact? The total refcount will probably go out of sync,
anything else?

The impact would be: objects incref/decref'ed by such a module would
be exposed to some of the performance penalties described earlier in
the PEP.  I expect the potential aggregate cost would be relatively
small.

If an extension DECREFs an immortal object, will it still match
_Py_IMMORTAL_REFCNT? How is that guaranteed?

It wouldn't match _Py_IMMORTAL_REFCNT, but the high bit of
_Py_IMMORTAL_REFCNT would still match.  That bit is what we would
actually be checking, rather than the full value.

It makes sense once you know _Py_IMMORTAL_REFCNT has two bits set. Maybe it'd be good to note that detail -- it's an internal detail, but crucial for making things safe.


What about extensions compiled with Python 3.11 (with this PEP) that use
an older version of the stable ABI, and thus should be compatible with
3.2+? Will they use the old versions of the macros? How will that be tested?

It wouldn't matter unless an object's refcount reached
_Py_IMMORTAL_REFCNT, at which point incref/decref would start
noop'ing.  What is the likelihood (in real code) that an object's
refcount would grow that far?  Even then, would such an object ever be
expected to go back to 0 (and be dealloc'ed)?  Otherwise the point is
moot.

That's exactly the questions I'd hope the PEP to answer. I could estimate that likelihood myself, but I'd really rather just check your work ;)

(Hm, maybe I couldn't even estimate this myself. The PEP doesn't say what the value of _Py_IMMORTAL_REFCNT is, and in the ref implementation a comment says "This can be safely changed to a smaller value".)



I'll omit the rest of the mail, it's clarifications (thank you!) & variations on the themes above.
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/TBV6IPD7JSW4JY2ROB4K7NERRSG2NSBS/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to