50859 - We should turn off JS_THREADSAFE if it is not really needed.

Status: RESOLVED WONTFIX

(SeaMonkey :: General, defect, P3)

Description

[John Bandhauer]

We should turn off JS_THREADSAFE it is not really needed. It costs us both space 
and time.

Comment 1

[John Bandhauer]

Attachment: a diff to assert JS is called ononly one thread

Comment 2

[John Bandhauer]

I ran with the above assert in place without problems. I got mail (using pop).

(NOTE the "#ifdef DEBUG_jband).

Comment 3

[Brendan Eich [:brendan]]

If we turn off JS_THREADSAFE, we can close 39373 as WONTFIX.  I think we'll want
at least one DEBUG-build safety, maybe in JS_EvaluateUCScriptForPrincipals (and
maybe JS_CallFunction*, whichever is the common subroutine).  Otherwise, some
naive Mozilla extender will run JS on a non-main thread and all heck will break
loose!

/be

Comment 4

[John Bandhauer]

[two emails. text from dougt...]

> vidur asks...
> I'm pretty clueless about xpinstall. Why does it spawn its own thread?

Yeah, we are.  XPInstall is your first (and only?) culprit.  It needs to process 
long install scripts in javascript.  This would block the UI thread. 

[the second email...]

XPInstall is a javascript based installing technology.  It adds its own objects 
into the javascript context so the user can add, query and remove files from the 
machine.  The script execution is linear, meaning that their is a start, a bunch 
of action, and a finalize.  During this execution, JS is being blocked in native 
code.  If this execution happened on the JS/DOM thread, xpinstall would not be 
able to have any UI and in fact would freeze the UI during the installations. 

Do we have any idea of the cost of threadsafety in mozilla?  Also, there may be 
others that are using js on different threads - I know about xpinstall because I 
am the one the put in on a new thread.
 

Comment 5

[Brendan Eich [:brendan]]

Trading for 46703 -- jband, do you want the dependent 39373 too?  Please say
yes, I'm about to give it to ya!

/be

Comment 6

[Doug Turner (:dougt)]

XPInstall is a javascript based installing technology.  It adds its own objects
into the javascript context so the user can add, query and remove files
from the machine.  The script execution is linear, meaning that their is a
start, a bunch of action, and a finalize.  During this execution, JS is being
blocked in native code.  If this execution happened on the JS/DOM thread,
xpinstall would not be able to have any UI and in fact would freeze the
UI during the installations. 

For embedding customers, this might be a win.  John, how much do we save in
space?

Comment 7

[Daniel Veditz [:dveditz]]

Don't forget javascript components -- when you write code that calls up another 
component it might just as easily be implemented in javascript for all you 
know. If you turn off JS_THREADSAFE then everyone has to make sure they don't 
ever use .js components for any reason in any case that might get called on a 
different thread.

Comment 8

[Mike Shaver (:shaver emeritus)]

We could, in theory, proxy JS component calls to the JS thread -- we have the
technology, if not necessarily the time or inclination

Feel the mighty wrath of LM_PostEvent!

Comment 9

[Doug Turner (:dougt)]

shaver, but then you would block UI while this proxied call happens.

Comment 10

[John Bandhauer]

[changing the bug Summary to include the word 'if' (as originally intended)]

On performance difference question: I have numbers in bug 43902 that show not 
having JS_THREDSAFE turned on makes JS about 3x faster. This is on code that 
spends almost all of its time doing locked property accesses. I don't know how 
this will translate into snappiness in the browser. I'll do a build with 
JS_THREADSAFE off and try to measure.

On xpinstall...

dougt, Do you have some real install scripts you can point me at?

What if we make changes to act like a modal dialog when running these things? We 
can run the JS on the main thread and spin an event loop during the long lasting 
activities (as they happen on another thread). Other windows get events, ours 
does not. JS happens on only one thread. What is wrong with this picture?

Comment 11

[Doug Turner (:dougt)]

There is an internal testing server which contains basic xpinstall test cases. 
(I don't know why it has not moved outside the firewall) Try: 
jimbob.mcom.com/tests/trigger3.html.  If that link doesn't work, try contacting
jimmy lee.  It is his machine.


If we have a better solution that works, I would be totally happy.  We ran into
threading problems when I first blocked the UI thread.  Since xpinstall blocks
for the entire duration of the install the entire product would appear crashed.  

Maybe another solution would to add "breathing" idles in xpinstall's native
routines to allow the xul ui to continue to function.

Comment 12

[Daniel Veditz [:dveditz]]

jband: go to the sweetlou products directory on ftp and in the daily build 
location you'll find update.html -- select navigator and launch it.

You could also go into the "xpi" subdir and download browser.xpi and examine 
the install.js script, but if you want to run it then update.html is the best 
way.

Comment 13

[Daniel Veditz [:dveditz]]

Jimmy and David did move their tests outside: 
http://www.mozilla.org/quality/smartupdate/xpinstall-trigger.html

But they are mostly smallish test cases for specific API's. Try doing something 
realistic like browser.xpi mentioned above -- especially on a Mac where it 
takes nearly ten minutes (which should be helped tremendously by jarred 
chrome).

Comment 14

[John Bandhauer]

Attachment: diff to build without JS_THREADSAFE

Comment 15

[John Bandhauer]

I added a patch of the changes I had to make to build without JS_THREADSAFE on 
Windows. Additional changes would be required for other platforms and the 
commercial tree. I'm not suggesting we check this in. I just didn't want to lose 
it if we decide to switch over in the future.

Comment 16

[John Bandhauer]

Datapoint from Dan Mosedale <[e-mail]>

FWIW, I'm writing the RDF datasource for LDAP as a JS component.  And
it gets callbacks from the LDAP connection thread when a directory
entry is received.  Right now, I'm processing the callbacks on the
LDAP connection thread in order to try and keep out of the way of the
layout engine.  

I could have the JS callbacks happen on the main() thread.  I'm not
really sure how this would affect performance, in part because
performance is already messed up because of event queue issues:
<http://bugzilla.mozilla.org/show_bug.cgi?id=50104>, and in part
because the datasource isn't quite far enough along yet.

Comment 17

[Brendan Eich [:brendan]]

Is there a separate bug to get Linux/x86 using thin-locks?  Who is gonna do it?

/be

Comment 18

[John Bandhauer]

No Linux/x86 specific thin lock bug, just bug 20357 where i showed that this 
would probably be a good thing.

maybe wtc or blizzard could you find a volunteer?

Comment 19

[John Bandhauer]

I tried to gather some numbers on the effects of compiling with and without 
JS_THREADSAFE on simple user tasks on a slower machine. I used a P133 runngin 
Win95 and a small stopwatch.

times in seconds (did 3 timings each - this shows significant margin of error):

                              safe       unsafe

time to startup using        13.93        13.44
mozilla.exe about:blank      13.85        13.64
time to done loading         13.97        13.55

new window using              4.57         4.61
ctrl-N showing mozilla.org    4.57         4.36
time to page visible          4.48         4.49

I also did some funky memory tests using taskinfo2000 showing "data KB"
only one test each (in KB)...

                              safe       unsafe
startup to about:blank         11.4       11.4
then goto tinderbox            12.78      12.85
then mozilla.org               12.4       12.24

cumulative doing startup to mozilla, typein 'cnn', click on tinderbox on 
toolbar, click on 'mail' button at botton of the screen (which brings me to the 
mail setup window on that machine (in KB):

                              safe       unsafe
                               18.8       18.9

This is all suspect, but it does not indicate dramatic speedups or memory 
savings by having JS_THREADSAFE off.

Comment 20

[vidur (gone)]

OK, I'll bring the feathers. Someone else supply the sticky road-coating 
material. I'll hang my head in shame at suggesting that building non-native code 
components isn't the right thing to do. I meant well.

I'd still like to see what the differences are for platforms on which we don't 
use thin locks.

Comment 21

[alla]

I've attached a patch for thin locks on linux x86 in bug 20357.

Comment 22

[alla]

Hmmmm.. My patch should really work on all x86 platforms that are using gcc.
Maybe i should just remove the defined(linux) thing and just use
defined(__GNUC__) && defined(__i386__). New patch comming up.

Comment 23

[John Bandhauer]

We seem to have decided that JS_THREADSAFE is inevitable and not hugely 
expensive.

Comment 24

[Brendan Eich [:brendan]]

See bug 54743, where I'll soon attach a patch that makes JS_THREADSAFE mostly
painless.

/be