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-

Description Darin Fisher (:fishd, Google) 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...
Comment 1 Darin Fisher (:fishd, Google) 2007-03-16 15:21:03 PDT
Created attachment 13672 [details]
patch
Comment 2 Darin Fisher (:fishd, Google) 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.
Comment 3 Alexey Proskuryakov 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.
Comment 4 Darin Fisher (:fishd, Google) 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.
Comment 5 Darin Adler 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()
Comment 6 Darin Adler 2007-03-19 20:39:05 PDT
because of
Comment 7 Darin Fisher (:fishd, Google) 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.
Comment 8 Darin Fisher (:fishd, Google) 2007-03-19 21:17:26 PDT
Created attachment 13714 [details]
patch: make inline and compare against 0

revised as requested
Comment 9 Darin Adler 2007-03-19 23:02:48 PDT
Comment on attachment 13714 [details]
patch: make inline and compare against 0

r=me
Comment 10 Maciej Stachowiak 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.
Comment 11 Maciej Stachowiak 2007-03-20 20:36:55 PDT
I fixed this in a different way.