ThreadIdentifier abstraction is inefficient

[Jens Alfke-4]

The implementation of the ThreadIdentifier abstraction (in wtf/
ThreadingPthreads.cpp) is pretty inefficient. This makes calls like  
WTF::currentThread() a lot slower than you'd expect — that call is  
showing up as a hot spot in Chromium's JS/DOM bindings, believe it or  
not.

I filed a bug on this: https://bugs.webkit.org/show_bug.cgi?id=30922

My first suggestion is just to typedef ThreadIdentifier to pthread_t  
(if pthreads are in use) and avoid the need for a mapping altogether.  
Any reason this wouldn't be a good idea? (If not, I have two other  
suggestions in the bug report.)

—Jens
_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Alexey Proskuryakov-4]

29.10.2009, в 13:31, Jens Alfke написал(а):

> I filed a bug on this: https://bugs.webkit.org/show_bug.cgi?id=30922
>
> My first suggestion is just to typedef ThreadIdentifier to pthread_t  
> (if pthreads are in use) and avoid the need for a mapping  
> altogether. Any reason this wouldn't be a good idea? (If not, I have  
> two other suggestions in the bug report.)

For those following along at home, this has been duped to <https://bugs.webkit.org/show_bug.cgi?id=25348 
 >, which already has some interesting discussion, and will  
undoubtedly have more.

- WBR, Alexey Proskuryakov

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Anton Muhin]

ens,

Somewhat aside from ThreadIdentifier.

Actually, at least for now, there should be no invocations of
WTF::currentThread() at least for the renderer---everything should run
in the same thread.  Some time ago I got rid of those checks for Nodes
(see DOMDataStore::weakNodeCallback).  I didn't do the same for other
types of objects as they didn't show up in the profiles.

May you tell me which test(s) you run?

And, overall, some time ago I failed to build Chromium w/ symbols (to
use Shark), may you teach me how to do that?

Thanks a lot for investigation and
yours,
anton.

On Thu, Oct 29, 2009 at 11:31 PM, Jens Alfke <snej@...> wrote:

> The implementation of the ThreadIdentifier abstraction (in
> wtf/ThreadingPthreads.cpp) is pretty inefficient. This makes calls like
> WTF::currentThread() a lot slower than you'd expect — that call is showing
> up as a hot spot in Chromium's JS/DOM bindings, believe it or not.
>
> I filed a bug on this: https://bugs.webkit.org/show_bug.cgi?id=30922
>
> My first suggestion is just to typedef ThreadIdentifier to pthread_t (if
> pthreads are in use) and avoid the need for a mapping altogether. Any reason
> this wouldn't be a good idea? (If not, I have two other suggestions in the
> bug report.)
>
> —Jens
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@...
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Jens Alfke-4]

On Oct 29, 2009, at 3:07 PM, Anton Muhin wrote:

> May you tell me which test(s) you run?

I saw this in Dromaeo's "DOM Query" test (the first one under "DOM  
Core Tests".) I believe it would also show up some of the other DOM  
tests, like DOM Modification, which also create and GC lots of DOM  
nodes.

> And, overall, some time ago I failed to build Chromium w/ symbols (to
> use Shark), may you teach me how to do that?

In chrome.xcodeproj, unfold the Targets list and find the "chrome_dll"  
target; then unfold that and click the final build step, "Postbuild  
'Strip If Needed'".
Open the inspector (File > Show Inspector).
Either delete both the strip and exit commands, or put "#"s in front  
of them.
Touch any source file in WebCore, then build.

—Jens
_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Dmitry Titov-2]

On Thu, Oct 29, 2009 at 3:10 PM, Anton Muhin <antonm@...> wrote:

ens,

Somewhat aside from ThreadIdentifier.

Actually, at least for now, there should be no invocations of
WTF::currentThread() at least for the renderer---everything should run
in the same thread.

That sounds right. Same for Chromium Worker processes. Some time ago we tried to run workers in the same process as renderer, or several workers in the Worker process using v8 locking. It didn't work well so we abandoned the idea. Seems like some good cleanup could help, nothing is as fast as not running the code! :-)

As for previous attempt to fix the issue, there were 2 main concerns (to recap):

- pthread_t is reused by some OSes between threads so comparing thread ids may give false matches in case we just use pthread_t as TheradIdentifier.

- Safari apparently uses WTF directly from JSC and changing binary interface and especially subtle protocol details will likely break nightly builds. Optimally, someone with visibility into both codebases could fix this with more confidence. Alternatively, perhaps such services as threading could be provided by the embedder via WebKit API rather then implemented in WTF - since embedders need access to the same time/thread services as the rest of WebKit.

Dmitry

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Anton Muhin]

@Dmitry: ok, so unless anyone else would volunteer (Jens?), I'd spend
some time next week looking how much we can get from it.

On Fri, Oct 30, 2009 at 1:29 AM, Jens Alfke <snej@...> wrote:
>
> On Oct 29, 2009, at 3:07 PM, Anton Muhin wrote:
>
>> May you tell me which test(s) you run?
>
> I saw this in Dromaeo's "DOM Query" test (the first one under "DOM Core
> Tests".) I believe it would also show up some of the other DOM tests, like
> DOM Modification, which also create and GC lots of DOM nodes.

Thanks a lot, Jens.

>> And, overall, some time ago I failed to build Chromium w/ symbols (to
>> use Shark), may you teach me how to do that?
>
> In chrome.xcodeproj, unfold the Targets list and find the "chrome_dll"
> target; then unfold that and click the final build step, "Postbuild 'Strip
> If Needed'".
> Open the inspector (File > Show Inspector).
> Either delete both the strip and exit commands, or put "#"s in front of
> them.
> Touch any source file in WebCore, then build.

Awesome, thank you another time.

yours,
anton.
_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev