mozilla-spidermonkey/js-lock-and-condition

Add higher-order abstractions on Lock to remove boilerplate

lars-t-hansen opened this issue · 7 comments

A pretty typical pattern is:

l.lock();
do something
do something else
l.unlock();

Observe especially how this pattern omits try...finally wrappers, which it probably should have to guarantee unlocking in the case of break, continue, return, or thrown exceptions. For the sake of convenience, and because JS likes this type of abstraction, we might introduce:

l.perform(function() {
  do something
  do something else
})

Similar ideas are possible for Cond, but the applicability is not really obvious.

EDIT: corrected typo.

One reason I haven't just written this code (it's trivial) is that I need to investigate whether a try...finally structure materially affects performance of common cases, notably, of a critical region that does not do something that will throw an exception. It would be good to test that in more than one browser, too.

Incorrectly closed by wrong bug reference in commit msg.

Sadly, in the SpiderMonkey shell, this (and its obvious variations):

try {
  this.lock()
  return thunk()
} finally {
  this.unlock()
}

is about 4x slower than this:

this.lock()
let r = thunk()
this.unlock()
return r

while this equivalent form is "only" 2.5x slower:

let r
try {
  this.lock()
  r = thunk()
  this.unlock()
} catch (e) {
  this.unlock()
  throw e
}
return r
hrj commented

How about having two variants of perform: safe and unsafe?

The safe variant would use try...finally, while the unsafe variant wouldn't do any exception handling. It would still be convenient than remembering to call unlock manually.

One could do that, but I really would prefer if try..finally wasn't that expensive. I consider that a bug. It's also a bug that simulating finally with try..catch is much faster than just using finally. (And for all I know those are only problems in Firefox, I haven't investigated yet.)

hrj commented

And for all I know those are only problems in Firefox, I haven't investigated yet.

FWIW, I have seen the same with v8 in Chromium and Node-js, and in the profiler, the reason for de-optimising is shown as "try block".

Filip Pizlo's strawman threaded JS proposal uses the syntax lock.hold(thunk) for what's called perform above.