tlrobinson / narwhal

[DEPRECATED] A JavaScript standard library, package manager, and more.
http://narwhaljs.org/
372 stars 16 forks source link

Upgrade Jack to use workers #51

Open kriszyp opened 14 years ago

kriszyp commented 14 years ago

Upgrade Jack to use WebWorkers for safer shared-nothing event-loop concurrency. I have implemented worker support in the simple and jetty servlet handler at: http://github.com/kriszyp/jack (I also upgraded the simple and servlet handlers that I rewrote to users workers and promises to roughly the current direction for JSGI 0.3. Figured I might as well while I was hacking on them.)

tlrobinson commented 14 years ago

Cool, this is exciting.

I'd like to hold off on transitioning to JSGI 0.3 for now, until it's actually finalized. We could just write a middleware adapter to convert 0.2 <-> 0.3.

I'd also like to retain the existing Simple and Servlet/Jetty handlers, in particular because I don't think Workers will work on AppEngine (no threads allowed).

Could we have separate simple-worker, servlet-worker, jetty-worker handlers, with the common parts (namely "env" object construction) factored out? Or maybe have it as an option, and add parameters to jackup to enable/disable workers, and set the number of worker threads.

This seems to remove the async support. I assume you just haven't gotten around to adding it back in?

Also, not a big deal, but there are some whitespace inconsistencies, especially in the -worker.js files. I'm trying to keep Narwhal/Jack close to Crockford's style guide (http://javascript.crockford.com/style1.html) with 4 spaces instead of tabs.

kriszyp commented 14 years ago

I'd like to hold off on transitioning to JSGI 0.3 for now, until it's actually finalized. We could just write a middleware adapter to convert 0.2 <-> 0.3.

Yeah, I suppose I got a little ahead of myself.

I'd also like to retain the existing Simple and Servlet/Jetty handlers, in particular because I don't think Workers will work on AppEngine (no threads allowed).

That makes sense. I've been thinking about how to maintain the shared-nothing concurrency in a servlet environment that controls the thread construction. Maybe I'll be able to come up with something there. Simple can't be used on GAE can it?

This seems to remove the async support. I assume you just haven't gotten around to adding it back in?

It should still be in there, but it now looks for the promise as the app response (rather than the return value of forEach) per the discussion on the ML.

Also, not a big deal, but there are some whitespace inconsistencies

Sorry about that, been following http://www.dojotoolkit.org/developer/StyleGuide for the last couple years, but I am trying to keep to the 4 space style with Narwhal stuff, will keep working on it.

tlrobinson commented 14 years ago

That makes sense. I've been thinking about how to maintain the shared-nothing concurrency in a servlet environment that controls the thread construction. Maybe I'll be able to come up with something there.

JackServlet might actually create a new context/scope for each thread. But if not, it certainly could.

Simple can't be used on GAE can it?

No, GAE just uses Servlets. But I'd still like to have the option of using the current Simple handler.

It should still be in there, but it now looks for the promise as the app response (rather than the return value of forEach) per the discussion on the ML.

Oh, I must have missed the conclusion of that discussion.

I'm not sure I like it, since it will break a lot more middleware.

Actually, it turns out that the AsyncResponse object I wrote yesterday might work correctly in both cases, since the object returned has the "then" method (even though it's only there because it's a convenient place for it):

http://github.com/tlrobinson/jack/blob/master/lib/jack/response.js#L160-195

I didn't notice the "when" call, which I see calls "then" (I was just looking for "then")

EDIT: Actually won't work since the new version expects the response object to be passed to the callback.

kriszyp commented 14 years ago

JackServlet might actually create a new context/scope for each thread. But if not, it certainly could

It doesn't right now (init() is only called once, not once for every thread), but maintaining a ThreadLocal for globals wouldn't be too difficult. We kind of lose the ability to do event-loops; we can execute queued events at the end of each request, but without async capabilities from the servlet engine (in the case GAE) it might be kind of pointless. Just having ThreadLocal globals would probably be enough.

I'm not sure I like it, since it will break a lot more middleware.

I don't mind switching it back, but we would be back in the situation of having to mutate an already returned response object in order asynchronously set different status codes and headers, which seems like it might break middleware in much harder to understand ways (and being able to async determine status seems like important functionality). Anyway, I am fine with either approach though.