[webkit-dev] Why are PassRefPtr<>s used as function parameters?

[Jeremy Orlow]

My thinking is that we should be consistent.  And given that passing a raw
pointer is equal to or faster than passing a PassRefPtr it seems to me that
we should just do that whenever ownership isn't being transfered.  Note that
this is what's done in the majority of cases I've seen and it's what the
RefPtr doc recommends, IIRC.

I don't see why the distinction between a callee simply using an object or
adopting it for future use is terribly interesting to the caller.

J

On Thu, Oct 29, 2009 at 3:55 PM, Drew Wilson <atwilson at google.com> wrote:

> Aside from the ref-counter munging, it still seems like there's value in
> using PassRefPtr as a signifier that a given API takes ownership of a
> pointer.
>
> So I'm not certain (in the case of HTMLNameCollection) that it'd be a
> better solution to change it not to take an ordinary pointer - yes, it would
> eliminate some ref churn, but at the cost of losing that connotation.
>
> A better solution (since the constructor is private) would be to make the
> constructor explicitly take a CollectionCache* parameter, thereby allowing
> us to directly pass the document straight through to the base class.
>
> -atw
>
>
> On Thu, Oct 29, 2009 at 3:31 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>
>>
>> On Oct 29, 2009, at 12:56 PM, Darin Adler wrote:
>>
>>  On Oct 27, 2009, at 10:55 AM, Jens Alfke wrote:
>>>
>>>  Looking at how refcounting is implemented in WebCore, I was surprised to
>>>> find that there are a lot of functions/methods that take PassRefPtr<>s as
>>>> parameters instead of regular pointers to those objects. I can't see any
>>>> benefit to this, and it adds the overhead of a ref() and deref() at every
>>>> call-site.
>>>>
>>>
>>> Have you read the RefPtr document? It is at <
>>> http://webkit.org/coding/RefPtr.html>.
>>>
>>> Only functions that take ownership of the passed-in objects should take
>>> PassRefPtr. This makes it so we can optimally handle the case where that
>>> object was just created and lets us hand off the reference instead of having
>>> to do a round of reference count thrash. If the function is going to be
>>> storing the value in a RefPtr, then the PassRefPtr does not introduce any
>>> reference count thrash even if the passed in object was not just created.
>>>
>>> It’s wrong for the HTMLNameCollection constructor to take a PassRefPtr
>>> for its document argument because it doesn’t take ownership of the document.
>>> So that one is definitely a mistake.
>>>
>>
>> However, its base class HTMLCollection can take ownership. The
>> HTMLNameCollection constructor misses out on the optimization though:
>>
>> HTMLNameCollection::HTMLNameCollection(PassRefPtr<Document> document,
>> CollectionType type, const String& name)
>>    : HTMLCollection(document.get(), type,
>> document->nameCollectionInfo(type, name))
>>    , m_name(name)
>> {
>> }
>>
>> Note that it passes document.get(), but also uses document after that so
>> it couldn't just pass document. Thus, it probably shouldn't take a
>> PassRefPtr (unless there is a clever way to fix it that I'm not seeing).
>>
>>  - Maciej
>>
>>
>>
>>
>>> It is true that PassRefPtr arguments can be tricky as you mentioned and
>>> the RefPtr document also mentions.
>>>
>>>   -- Darin
>>>
>>> _______________________________________________
>>> webkit-dev mailing list
>>> webkit-dev at lists.webkit.org
>>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091029/a5319d00/attachment.html>