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

bpo-35514: Enhanced the explanation on reference count details #11277

Closed
wants to merge 5 commits into from

Conversation

bombs-kim
Copy link
Contributor

@bombs-kim bombs-kim commented Dec 21, 2018

@birkenfeld

When I read the reference count details section of the docs first time, I found it hard to grasp what transferring of ownership means concretely, which is an important and repeating concept throughout the docs. Some parts were confusing. For example,

When a function passes ownership of a reference on to its caller, the
caller is said to receive a new reference

This part seems to explain what is to receive a new reference, in terms of ownership passing. It may have had to do the opposite, which is to explain what a transferring of ownership is in terms of reference count changes because transferring of ownership is a high level concept and changing a reference count is a concrete operation. I fixed the documentation based on this idea.

https://bugs.python.org/issue35514

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Dec 21, 2018
eamanu
eamanu approved these changes Dec 21, 2018
Copy link
Contributor

@eamanu eamanu left a comment

LGTM

reference count of it, the function is said to *give* ownership of a new
reference to its caller and the caller is said to *own* the reference. When a
function returns an object without changing the reference count of it, the
caller is said to *borrow* the reference. Nothing needs to be done for a
borrowed reference.

Conversely, when a calling function passes in a reference to an object, there
are two possibilities: the function *steals* a reference to the object, or it
Copy link
Contributor

@eamanu eamanu Dec 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you delete the double whitespace on .. *steals* a reference...

Copy link
Contributor Author

@bombs-kim bombs-kim Dec 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can😄

@csabella csabella requested a review from pablogsal May 29, 2019
@csabella
Copy link
Contributor

csabella commented May 29, 2019

Asking @pablogsal for review as this seems closely related to his PyCon talk. 🙂

Copy link
Member

@pablogsal pablogsal left a comment

Thanks for working on this!

I left some comments

caller). Let's look at different types of reference ownership passings in more
detail.

When a function returns an object and effectively increases the
Copy link
Member

@pablogsal pablogsal Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function does not increase the reference count when returning. Returning is just returning. The reference count is increased explicitly or implicitly on creation. Normally, a function returns something that someone else has created (and therefore increased the refcount by 1 - or alternatively is born with a refcount of 1). The function is transfering ownership of the existing refcount, not increasing it (as you mention somehow in the next sentences. I think this could be clearer.

Copy link
Contributor Author

@bombs-kim bombs-kim Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew that returning itself doesn't affect refcount and that is why I used the word "effectively".
I guess it can be clearer if I say
"When a function increases the reference count of an object and returns it, the function..."
But maybe it's not perfect yet.

I thought a function can increase the refcount of some object in a sense that the reference count can increase during that function calling. If I understood you correctly, I can't say a function increases or doesn't increase the refcount of some object because the function is only transferring the ownership of some existing object, right?

Do you think these are also wrong to say?

  • When a function returns an object without changing the reference count of it, the caller is said to borrow the reference.
  • (You comment below) The called function has two choices: Increase the refcount - take ownership / Don't increase the refcount

Copy link
Member

@pablogsal pablogsal Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hummm, I think there may be some confusion because I was not clear in my wording. My comment below applies only to the case when you pass something to a function and then the function can get ownership (increase the refcount) or stealing (don't increase the refcount).

This paragraph that we are commenting here applies to the case when the function returns. In that case, the function can tell you "here is my object, you can use it but is mine and can die unless you take ownership of it (increasing the refcount). But on the other side if you don't need it anymore you don't need to do something because is still mine" or conversely "here is a new object for you, you need to remember to decrease the reference when you no longer need it".

Copy link
Contributor Author

@bombs-kim bombs-kim Jun 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, now I see those two cases are different and let's only talk about this paragraph that we are commenting.

First, please be reminded of the purpose of this PR. If one reads this docs for the first time he has no idea what transferring of ownership actually means and he needs an explanation like this

Suppose a caller called a callee

  • Owning a reference: the caller received an object from the callee which had effectively increased the reference count of the object it returned by 1 during function calling
  • Borrowing a reference: the caller received an object from the callee which hadn't changed the reference count of the object it returned during function calling

Can you see why I keep talking about change of ownership in terms of increasing and decreasing of reference count now?

The only thing I want to make sure here is if it makes sense to say a function increases or doesn't increase the refcount of the object it returns. Can't I say PyLong_FromLong(1L) increases the reference count of the object it returns, which is int(1)? If not, how do you think I can explain it concretely? I can't simply say the caller receives an ownership of the object PyLong_FromLong returns because it's only a circularly explanation and doesn't help readers.

Copy link
Member

@pablogsal pablogsal Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Owning a reference: the caller received an object from the callee which had effectively increased the reference count of the object it returned by 1 during function calling

But this is incomplete IMHO, you can receive some object via a borrow reference, increase it and now you own it. Ownership is linked to the duty of decreasing the reference when you are down, not on how did you obtain the object.

Copy link
Contributor Author

@bombs-kim bombs-kim Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean. However, if I defined owning a a reference according to your suggestion, that concept would not be mutually exclusive with borrowing a reference. This may not be a good categorization and I expect it will only confuse readers.

I you don't like the word own, maybe we can use take instead because it has a more specific meaning in that it implies transferring of ownership. In addition, this doc also defines giving an ownership and it's more natural to talk about give and take than own and take.

Copy link
Member

@pablogsal pablogsal Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Owning a reference is a contract (you are responsable to clean it) while borrowing a reference is a verb: is an action that ends with you not owning the reference. But then you can own it by increasing the reference count.

Copy link
Contributor Author

@bombs-kim bombs-kim Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I understand what you mean, but that was not the point of my argument. It seems this conversation is getting more confusing and I think I can just revert this part back to the old version.

responsible for it any longer.
Conversely, when a calling function passes in a reference to an object, there
are two possibilities: the function *steals* a reference to the object, or it
does not. If a called function decreases the reference count of an object, it
Copy link
Member

@pablogsal pablogsal Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not clear to me or is directly wrong. The called function has two choices:

  1. Increase the refcount - take ownership
  2. Don't increase the refcount - stealing ownership

In any case the function never decreases the reference count.

Copy link
Contributor Author

@bombs-kim bombs-kim Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the correction🙏

Copy link
Contributor Author

@bombs-kim bombs-kim Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pablogsal After studying some functions, I learned that functions that are considered to steal ownerships do decrease refcount. Take a look at this example PyTuple_SetItem

Copy link
Member

@pablogsal pablogsal Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After studying some functions, I learned that functions that are considered to steal ownerships do decrease refcount.

I have a reference, I passed to some function and it steals my reference by placing it into a tuple and not doing anything with my reference count. The function says that is "stealing" the reference: in this example, nobody is decrementing or incrementing anything. Now is somebody else's job to manage that object. In this example, this hypothetical function has stolen the reference from me without altering the refcount.

Copy link
Contributor Author

@bombs-kim bombs-kim Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really sorry for the hassle and I'm afraid I can't fully understand what you are saying here. However, I think I get this at least. "Stealing a reference does not guarantee instant decreasing in refcount". I will change the doc accordingly.
But please note that, for example, PyTuple_SetItem does decrease the refcount of an passed-in object in some cases. I still think that stealing a reference can be understood as a contract thatPy_DECREF would be called on the object of interest by the callee or anybody else who is not the caller in the future.

Doc/c-api/intro.rst Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Jun 3, 2019

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@pablogsal
Copy link
Member

pablogsal commented Jun 3, 2019

Thanks @csabella for the ping :)

@JulienPalard JulienPalard requested a review from pablogsal Sep 13, 2019
@csabella
Copy link
Contributor

csabella commented Jun 12, 2020

Since there was an additional commit after the last comments, I believe this PR is waiting for review. @pablogsal

@JelleZijlstra
Copy link
Member

JelleZijlstra commented May 8, 2022

This has a merge conflict now. I'm not sure the change is necessary at all: the existing text seems clear to me, and we can't realistically address all possible user confusion.

@iritkatriel
Copy link
Member

iritkatriel commented Jun 22, 2022

This has a merge conflict now. I'm not sure the change is necessary at all: the existing text seems clear to me, and we can't realistically address all possible user confusion.

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants