Open Bug 786819 Opened 12 years ago Updated 1 year ago

reduce number of relocations required by new bindings data structures

Categories

(Core :: DOM: Bindings (WebIDL), defect, P3)

Product:
Core
Component:
DOM: Bindings (WebIDL)
Type:
defect

Tracking

()

People

(Reporter: froydnj, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [overhead:noted]) 

The new bindings could use some of the same treatment quickstubs received in bug 711563.  Some new JS interfaces will be required, though...
Do you know offhand which particular parts of the new bindings are getting relocated?  I'm happy to rejigger things, but I haven't had a chance to sit down and figure out where the relocations are coming from...  I'm also not sure what the effective relocation-reduction techniques are.
(In reply to Boris Zbarsky (:bz) from comment #1)
> Do you know offhand which particular parts of the new bindings are getting
> relocated?

Sure.  For instance, WebGLRenderingContextBinding.cpp:

static JSFunctionSpec sMethods_specs[] = {
  JS_FNINFO("getContextAttributes", genericMethod, &getContextAttributes_methodinfo, 0, JSPROP_ENUMERATE),
  ...

That's three relocations:

- "getContextAttributes";
- genericMethod,
- getContextAttributes_methodinfo.

You could avoid the first and third pretty trivially.

Notice that you have one relocation for every function due to the genericMethod pointer.  I assume you mean to change this at some point so each function has an individual JSNative?  If not--or even if you do plan on doing this, but only for a select subset of functions--you could save on relocations by having a table of JSNatives and indexing into that, thus sharing relocations.  Whether that's really worth it or not depends on how much sharing you can do, of course...

static Prefable<JSFunctionSpec> sMethods[] = {
  { true, &sMethods_specs[0] },

That's one relocation for the &sMethods_specs[0].  And so on for all instances of Prefable arrays generated by the bindings.

static JSPropertySpec sAttributes_specs[] = {
  { "canvas", 0, JSPROP_SHARED | JSPROP_ENUMERATE | JSPROP_NATIVE_ACCESSORS, {(JSPropertyOp)genericGetter, &canvas_getterinfo}, JSOP_NULLWRAPPER},

That's three relocations:

- "canvas";
- genericGetter;
- &canvas_getterinfo.

It'd be five with information for the setter (e.g. CSS2PropertiesBinding.cpp).  Same comments apply vis a vis genericGetter and getterinfos as for the function specs earlier.

static ConstantSpec sConstants_specs[] = {
  { "DEPTH_BUFFER_BIT", UINT_TO_JSVAL(256) },

One for the string here.

"Better" JS interfaces could be built to avoid some relocations with JSJitInfo and the above, though I don't know how we feel about having semi-private interfaces like that; I wouldn't think interfaces that enable avoiding relocations are necessarily good as public interfaces.  It seems that we're repeating a lot of information in JSJitInfos that we should be able to compress, too.

Those are all the big ones I notice immediately; I haven't combed through looking for others, but I bet that's most of them (other than things like JSClass/DOMJSClass, which are basically vtables and hard to fix).

Generating some of the tables--particularly string tables--for the above across all bindings at once would be helpful for space savings, too.
> You could avoid the first and third pretty trivially.

For the string, I guess I could mush up all the strings for the file into a single large static const char[] and then use offsets into that?

For the methodinfos, same deal with an array of them and indexing into it?

What you said about &sMethods_specs[0] needing a relocation makes it sound like "no" for both of the questions above...

> assume you mean to change this at some point so each function has an individual
> JSNative? 

Nope.  We used to have that; we moved to this setup to save codesize.  I'm not sure what you mean about a table here.  There are basically three jsnatives (typically; there might be two or one in some cases) per translation unit: genericGetter, genericSetter, genericMethod.  How would I use the table approach for them?

Generating things across all bindings is ... doable if we have to.  It's a bit suboptimal, not least because it doesn't parallelize well.  But I'd like to understand how one actually goes above avoiding relocations first, before I worry about coalescing across bindings.
(In reply to Boris Zbarsky (:bz) from comment #3)
> > You could avoid the first and third pretty trivially.
> 
> For the string, I guess I could mush up all the strings for the file into a
> single large static const char[] and then use offsets into that?

Right.  You can usually use uint16_t for indices, which saves space and packs better with other things than pointers, too.

> For the methodinfos, same deal with an array of them and indexing into it?

Right.  Blue-sky wish is sharing most of the methodinfos/getterinfos too.

> What you said about &sMethods_specs[0] needing a relocation makes it sound
> like "no" for both of the questions above...

I'm not sure how that follows.  The two cases above are using offsets into a table, not the address of the table element at that offset, as in the &sMethods_specs[0] case.

> > assume you mean to change this at some point so each function has an individual
> > JSNative? 
> 
> Nope.  We used to have that; we moved to this setup to save codesize.  I'm
> not sure what you mean about a table here.  There are basically three
> jsnatives (typically; there might be two or one in some cases) per
> translation unit: genericGetter, genericSetter, genericMethod.  How would I
> use the table approach for them?

I mean that instead of:

  { ..., genericGetter, ... },
  { ..., genericGetter, ... },
  { ..., genericGetter, ... },

you could have:

typeof(genericGetter) jsnativeFuns[] = { genericGetter, ... };

  { ..., /*index into jsnativeFuns*/0, ... },
  { ..., 0, ... }, ...

which saves relocations and may also enable better packing of structures.  That may be too clever for the first cut, though.

> Generating things across all bindings is ... doable if we have to.  It's a
> bit suboptimal, not least because it doesn't parallelize well.  But I'd like
> to understand how one actually goes above avoiding relocations first, before
> I worry about coalescing across bindings.

Totally.  The big win will be avoiding the relocations in the first place, and then maximally sharing things in each *Binding module; cross-module sharing is probably more of a blue-sky thing.
> The two cases above are using offsets into a table

Ah, so the JSFunctionSpec would store a number, not a string?

That would require JSAPI changes, basically: that struct is passed into the JS engine and it expects a const char* there, obviously.

Same for the methodinfos.

I guess you did say that in comment 0...

I could stop using the relevant JSAPI and just manually define everything myself, in which case I can do my own indexing into an array at the definition site, but that would mean a huge number more function calls to set up a DOM prototype...  Maybe that's ok.
How hard would it be to stick all this code somewhere *inside* the engine?
Probably not terribly hard.

Except the Prefable bit, of course.

Note that in the common case (nothing is prefable), the one relocation seems somewhat unavoidable...
(In reply to Boris Zbarsky (:bz) from comment #5)
> > The two cases above are using offsets into a table
> 
> Ah, so the JSFunctionSpec would store a number, not a string?

Yes, at which point we'd have to call it a DOMJSFunctionSpec or somesuch.

> I could stop using the relevant JSAPI and just manually define everything
> myself, in which case I can do my own indexing into an array at the
> definition site, but that would mean a huge number more function calls to
> set up a DOM prototype...  Maybe that's ok.

FWIW, quickstubs doesn't use the "bulk" calls (JS_DefineFunctions), but instead defines them one-by-one.  It's not obvious to me that:

  JS_DefineFunctions(...)

is any faster than:

  for (i = 0; i < ...) JS_DefineFunction(...)

since presumably JS_DefineFunctions has to have the same loop *somewhere*.
It has a loop, but the loop doesn't call JS_DefineFunction.  It calls a lower-level method, so you skip several levels of indirection, including the overhead of cross-module calls for each function...

But yes, I know quickstubs does this manually.  Again, I'd be willing to try it, esp if we come up with a good way to measure the performance.  That last is kinda hard until we have more protos hooked up, sadly.
But maybe setting up a single proto takes long enough we can measure it using performance.now() ?
  <script>
    var start = performance.now();
    WebGLRenderingContext;
    var stop = performance.now();
    document.write(stop - start);
  </script>

Worth measuring that as-is and with manual JS_DefineFunctions calls.  As-is is about 0.4ms.
For what it's worth, profiling that testcase, if I trust my profiler, shows about 2-1 time ratio between JS_DefineProperty (for the constants) and JS_DefineFunctions (for the methods).  There are 295 constants and 137 methods.  So it might all be a wash.  The biggest cost, really, is atomizing all those strings.

So another option is to switch to JS_DefinePropertyById and use our existing id arrays to implement that.  Should be a perf win in addition to reducing relocations!
Depends on: 1451516
Priority: -- → P3
This is a lower priority for memshrink-content, we think a fork server will help more with relocations than chasing down individual causes. This could still be a perf win and in install size win.
Whiteboard: [overhead:noted]
Component: DOM → DOM: Core & HTML
Component: DOM: Core & HTML → DOM: Bindings (WebIDL)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.