After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 630964 - Optimize pushing the current context
Optimize pushing the current context
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2010-09-29 22:20 UTC by Owen Taylor
Modified: 2010-09-30 14:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Optimize pushing the current context (3.34 KB, patch)
2010-09-29 22:20 UTC, Owen Taylor
committed Details | Review
Use JS_SetRuntimePrivate() rather than GDataset for runtime data (7.21 KB, patch)
2010-09-29 22:45 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2010-09-29 22:20:18 UTC
Here's what I think is a working optimization of the same-frame optimization

(Getting the accounting wrong in a similar case for JS_BeginRequest is why we started
on this whole odyssey - Spidermonkey had code for nested BeginRequest for different
contexts, it just was buggy...)
Comment 1 Owen Taylor 2010-09-29 22:20:19 UTC
Created attachment 171378 [details] [review]
Optimize pushing the current context

For gjs_runtime_push_context() keep track of the number of times
an identical context has been pushed. This allows avoiding allocating
a GSList node for the most common case of repeatedly pushing the
default context.
Comment 2 Owen Taylor 2010-09-29 22:45:55 UTC
Created attachment 171379 [details] [review]
Use JS_SetRuntimePrivate() rather than GDataset for runtime data

GDataset is slow - it requires thread locking and hash table lookups.
Since we don't support foreign JSRuntimes at the moment we can use
JS_SetRuntimePrivate() which will have better performance.

The public API functions gjs_runtime_get/set_data() are removed; if
they were being used outside of GJS the users will have to track their
data themselves.
Comment 3 Colin Walters 2010-09-30 14:35:34 UTC
Review of attachment 171378 [details] [review]:

OK
Comment 4 Colin Walters 2010-09-30 14:36:21 UTC
Review of attachment 171379 [details] [review]:

Your patch series didn't remove the runtime constructor argument to GjsContext, did it?  If not it will need to be removed.  Otherwise OK.
Comment 5 Owen Taylor 2010-09-30 14:44:16 UTC
(In reply to comment #4)
> Review of attachment 171379 [details] [review]:
> 
> Your patch series didn't remove the runtime constructor argument to GjsContext,
> did it?  If not it will need to be removed.  Otherwise OK.

What's legal with my patch series is:

 Create a GjsContext not specifying a runtime
 Creating more GjsContexts using the runtime from the first context
  (You actually have to do JS_GetRuntime(gjs_get_native_context(gjs_context))
   to get the runtime - so sort of an API gap.)

Which is what you'd want to do in the hypothetical applet or threads case. Though having a GjsRuntime object would be cleaner. Trying to pass in a runtime that wasn't create by GJS will die with a gjs_fatal() error.