Bug 22030 - Make EventNames usable from multiple threads
Summary: Make EventNames usable from multiple threads
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-01 08:03 PDT by Alexey Proskuryakov
Modified: 2008-11-04 02:48 PST (History)
1 user (show)

See Also:


Attachments
proposed patch (238.00 KB, patch)
2008-11-01 15:36 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
proposed patch (235.80 KB, patch)
2008-11-01 15:38 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2008-11-01 08:03:44 PDT
Event names are used in classes that will be used in worker threads. It is probably impractical to attempt making atomic strings fully thread safe (usable from multiple threads at once), so each thread needs to have its own stringTable, and its own instance of global strings.

Patch forthcoming.
Comment 1 Alexey Proskuryakov 2008-11-01 15:36:04 PDT
Created attachment 24836 [details]
proposed patch
Comment 2 Alexey Proskuryakov 2008-11-01 15:38:37 PDT
Created attachment 24837 [details]
proposed patch

Oops, sent before saving...
Comment 3 Darin Adler 2008-11-03 09:12:58 PST
Comment on attachment 24837 [details]
proposed patch

Any uses in WebKit outside of the "mac" and "win" directories?

Typo "Atimic" in one place.

What performance impact does this change have?

Why is the EventNames constructor public? Why is "int dummy" a member of EventNames? Just to allow the initialization macro to work? I think both the constructor and the dummy member should be private.

> : dummy (0)

There should be a space after the word "dummy" here?

> // Initialization is not thread safe, so this function must be called from the main thread first.

Why not assert this?

r=me
Comment 4 Alexey Proskuryakov 2008-11-03 09:51:34 PST
(In reply to comment #3)
> (From update of attachment 24837 [details] [edit])
> Any uses in WebKit outside of the "mac" and "win" directories?

No, grepping didn't find anything else.

> What performance impact does this change have?

No change on PLT for me. Should I test anything else? I worried about a PLT regression, because this changed AtomicString construction and destruction, but event dispatching is probably not that hot - or is it?

> Why is "int dummy" a member of EventNames? Just to allow the initialization macro
> to work?

Yes, that's the reason.
Comment 5 Alexey Proskuryakov 2008-11-04 02:48:08 PST
Committed revision 38094.