WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
17256
eliminate default ref. count of 0 in RefCounted class
https://bugs.webkit.org/show_bug.cgi?id=17256
Summary
eliminate default ref. count of 0 in RefCounted class
Darin Adler
Reported
2008-02-09 06:46:01 PST
To speed up creation of RefCounted objects, we want to change it so objects start with a ref count of 1 rather than starting with a ref count of 0 and then incrementing it to 1. To do this we need to change RefCounted clients, one at a time, to ask for a count of 1.
Attachments
add explicit initialization of RefCounted to 0 almost everywhere
(86.95 KB, patch)
2008-02-09 06:49 PST
,
Darin Adler
no flags
Details
Formatted Diff
Diff
patch for next step -- actually remove the default
(2.15 KB, patch)
2008-02-10 11:33 PST
,
Darin Adler
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2008-02-09 06:49:52 PST
Created
attachment 19016
[details]
add explicit initialization of RefCounted to 0 almost everywhere
Eric Seidel (no email)
Comment 2
2008-02-10 10:55:01 PST
Comment on
attachment 19016
[details]
add explicit initialization of RefCounted to 0 almost everywhere r=me. It seems like changing everything to start with a ref count of 1 is a lot of work for little gain. The malloc() should always dwarf the ref() call I would think. Then again, maybe the parser could benefit from this? How does one change RefPtr to take advantage of such a change? (Since a stated goal has long been to get rid of most raw pointers in WebCore and instead us scoped/smart pointers like RefPtr.)
Darin Adler
Comment 3
2008-02-10 11:03:07 PST
(In reply to
comment #2
)
> It seems like changing everything to start with a ref count of 1 is a lot of > work for little gain. The malloc() should always dwarf the ref() call I would > think.
I once thought this too, but recent measurements seem to indicate otherwise. For example, Geoff Garen got a 1% speedup with <
http://trac.webkit.org/projects/webkit/changeset/30042
>, which is also something that happens only once per object. I predict this will be well worth it; we should start seeing speed-ups soon. This also ends up with a much more elegant usage model. We don't want you to be able to create an object with a ref count of 0. That's just a recipe for leaking. Instead we make objects and put them in PassRefPtr right away. It'll be less elegant during the transition.
Darin Adler
Comment 4
2008-02-10 11:04:15 PST
Comment on
attachment 19016
[details]
add explicit initialization of RefCounted to 0 almost everywhere Eric reviewed and said r=me, but forgot to set review+.
Darin Adler
Comment 5
2008-02-10 11:06:04 PST
(In reply to
comment #2
)
> How does one change RefPtr to take advantage of such a change?
You don't have to change RefPtr. You just call adoptRef on the result of new rather than putting it in a RefPtr or PassRefPtr without adoptRef. Here's an example: <
http://trac.webkit.org/projects/webkit/changeset/30109
>. The short-term scaffolding is the need to add explicit initialization of RefCounted. Once the default is changed to 1 we can remove those again.
Darin Adler
Comment 6
2008-02-10 11:28:31 PST
Committed revision 30122. This task won't be completely done until I take out the RefCounted default of 0 and we get rid of the last few clients. Then I'll change the default to 1 for the future.
Darin Adler
Comment 7
2008-02-10 11:28:51 PST
Comment on
attachment 19016
[details]
add explicit initialization of RefCounted to 0 almost everywhere Clearing review flag since this was landed.
Darin Adler
Comment 8
2008-02-10 11:33:06 PST
Created
attachment 19043
[details]
patch for next step -- actually remove the default
Eric Seidel (no email)
Comment 9
2008-02-10 11:38:38 PST
Comment on
attachment 19043
[details]
patch for next step -- actually remove the default r=me
Darin Adler
Comment 10
2008-02-14 09:31:28 PST
http://trac.webkit.org/projects/webkit/changeset/30133
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug