Bug 13096

Summary: make it possible to enable USE(MULTIPLE_THREADS) on other platforms
Product: WebKit Reporter: Darin Fisher (:fishd, Google) <fishd>
Component: JavaScriptCoreAssignee: Maciej Stachowiak <mjs>
Status: RESOLVED FIXED    
Severity: Minor CC: ap
Priority: P4    
Version: 523.x (Safari 3)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch: make isMainThread be a static member function
darin: review-
patch: make inline and compare against 0 mjs: review-

Darin Fisher (:fishd, Google)
Reported 2007-03-16 14:56:36 PDT
make it possible to enable USE(MULTIPLE_THREADS) on other platforms Currently, it is trivial to provide a platform-specific JSLock.cpp file and to specify the macros that dtoa.cpp requires when USE(MULTIPLE_THREADS) is specified. However, collector.cpp needs some modifications to make it portable. Patch coming up...
Attachments
patch (3.32 KB, patch)
2007-03-16 15:21 PDT, Darin Fisher (:fishd, Google)
no flags
patch: make isMainThread be a static member function (3.39 KB, patch)
2007-03-16 15:29 PDT, Darin Fisher (:fishd, Google)
darin: review-
patch: make inline and compare against 0 (3.39 KB, patch)
2007-03-19 21:17 PDT, Darin Fisher (:fishd, Google)
mjs: review-
Darin Fisher (:fishd, Google)
Comment 1 2007-03-16 15:21:03 PDT
Darin Fisher (:fishd, Google)
Comment 2 2007-03-16 15:29:42 PDT
Created attachment 13674 [details] patch: make isMainThread be a static member function ap pointed out that it would ease porting if isMainThread were a static member function.
Alexey Proskuryakov
Comment 3 2007-03-17 13:23:23 PDT
(In reply to comment #2) > ap pointed out that it would ease porting if isMainThread were a static member > function. Actually, I just asked how this patch makes it possible to enable USE(MULTIPLE_THREADS) on other platforms if it doesn't implement isMainThread() for those. Morgan's answer was that it can be specified using -D.
Darin Fisher (:fishd, Google)
Comment 4 2007-03-18 16:24:17 PDT
True, true... anyways, after we discussed that, I decided that it would be better to specify it as a static member function. That seems more consistent with how other code in webkit is made portable.
Darin Adler
Comment 5 2007-03-19 20:38:50 PDT
Comment on attachment 13674 [details] patch: make isMainThread be a static member function This function is quite performance-critical. Thus I'd prefer that the Darwin versino of the function be marked inline. + return pthread_main_np() == 1; That's incorrect. != 0 would be correct. review- before of the incorrect call for pthread_main_np()
Darin Adler
Comment 6 2007-03-19 20:39:05 PDT
because of
Darin Fisher (:fishd, Google)
Comment 7 2007-03-19 21:05:08 PDT
> This function is quite performance-critical. Thus I'd prefer that the Darwin > versino of the function be marked inline. Oh, does the Mac generate relocation entries for static class functions? I know that ELF (Linux) does so, which really sucks. On Windows, such a static class member function would just be entirely optimized away. Is that really not the case on Mac? > + return pthread_main_np() == 1; > > That's incorrect. != 0 would be correct. OK. I read some documentation that claimed that -1 indicated an error, 0 indicated false, and 1 indicated true. I agree that != 0 matches existing usage, so that does seem better.
Darin Fisher (:fishd, Google)
Comment 8 2007-03-19 21:17:26 PDT
Created attachment 13714 [details] patch: make inline and compare against 0 revised as requested
Darin Adler
Comment 9 2007-03-19 23:02:48 PDT
Comment on attachment 13714 [details] patch: make inline and compare against 0 r=me
Maciej Stachowiak
Comment 10 2007-03-20 04:27:30 PDT
I don't think this should be landed, since it conflicts with my patch that will make this one unnecessary. Changing to r-. Discussed with submitter.
Maciej Stachowiak
Comment 11 2007-03-20 20:36:55 PDT
I fixed this in a different way.
Note You need to log in before you can comment on or make changes to this bug.