Re: gjs review

[Alexander Larsson <alexl redhat com>]

On Wed, 2008-10-15 at 13:31 -0400, Havoc Pennington wrote:
> Hi,

I see you replied to the wrong copy. CC:ing language-bindings on the
reply. :)

> On Tue, Oct 14, 2008 at 6:15 AM, Alexander Larsson <alexl redhat com> wrote:
> > I want a GJsValue GObject that lets you easily handle a js value from C
> > code.
> 
> This may be obvious if I just read the gscript code, but should we use
> GValue here?

Oh, thats a completely different thing. GJsValue is a nice c-side view
of a jsval. Its automatically rooted, so you can easily handle it
however you want, and it lets you check what kind of js type the value
is, and if its an object/function/array/gobject-wrapper js value you can
do type specific operations on them (like setting a named property of an
object to another GJsValue). 

GValue implies some form of conversion from the js universe to a common
type/value universe whereas a GJsValue is a view into the JS engine.

> > Why is there an immutable global property "window" always set? That
> > seems very web:ish, and might not always be what you want.
> 
> I'm not sure, I did this very early on. I think just trying to name
> the global object; otherwise it doesn't have a name so can't be
> reached easily unless it's the top of the scope chain. And from there,
> "why not name it what most people expect it to be named on the web"

Well, "window" is also a quite common variable name, so hogging it with
something that might not be useful is kinda weird. And if you want to
have this in your app, it should be easy to use the API to add it.

> > Why does GJsContext default to a non-shared runtime object?
> 
> GjsContext is "designed" on the premise that you only want one stack
> per runtime. This is a simplification that made sense to me for our
> app. I'm not sure multiple contexts are terribly useful without
> multiple threads, and gjs is not at all thread safe.

On the contrary, they are very useful for many apps, even if you don't
use threads. The main reason is that they can give you multiple global
objects. In fact, this is how Mozilla does it. Each window has its own
context and thus global object ("window" as discussed above). This imho
makes very much sense when scripting a document centric application.
You'd set the document being edited as the global, making it easy to
access document specific stuff.

Actually, even if you want to use threads having each context have its
own runtime doesn't buy you anything, because spidermonkey does have
global data that is not part of the runtime (see the thread docs). So,
by not sharing the runtime the only advantage you get is higher resource
usage, limitations on sharing objects between contexts, and risks of
weird behaviour if you accidentally share an object. 

When designing this part of GScript i actually talked to some of the
spidermonkey developers on irc about how to best expose this in an API
like mine, and they said the best way was to use a shared runtime and
allow multiple contextes. In fact, even they could not cite a reason for
ever using multiple runtimes.

If you really want to be threadsafe, see this page for what you need to
do:
http://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_THREADSAFE

> For a future-safe API, we should probably expose context and runtime
> separately (not sure how webkit does it, if trying to abstract that).
> A bunch of spidermonkey API takes a context when it really should take
> a runtime, which makes spidermonkey sort of confusing. They have some
> new API that operates on the runtime instead, and seem to be
> deprecating some of the stuff taking a context that should have taken
> a runtime.

I don't really see any reason to expose the runtime object at all in a
higher level API. Just use a shared one internally. Can you mention even
a single advantage that multiple runtimes buy you?

> JSContext is sort of a weird mess in other ways too. The global object
> set on it is just the *default* global object when creating new scope
> chains; it's ignored when evaluating expressions, instead the last
> object on the scope chain is the global object at runtime. So the way
> to get the global object that will be seen from JS is to call
> JS_GetParent() repeatedly, *not* to call JS_GetGlobalObject()
> 
> When a closure is evaluated, the scope chain on the context gets
> swapped out for example, and the global object might not match the one
> in the context.

I'm not sure exactly why its like that, but we don't have to ever call
JS_EvaluateScript() or JS_ExecuteScript() with any other scope than the
global one. But this whole area is kinda confusing and we should perhaps
research it a bit more...

> > gjs_context_eval uses JS_EvaluateScript, so string can only be ascii. It
> > should properly convert to UTF16 and use JS_EvaluateUCScript so you can
> > e.g. have utf8 strings in your code.
> 
> Sounds appropriate.

Also, maybe we want to expose a script type to allow JS_ExecuteScript()
for repeated evaluation of a script.