503157 - TM: avoid integer division in NewGCThing path

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Andreas Gal :gal]

Attachment: patch

This was originally discovered by Gregor.

Comment 2

[Andreas Gal :gal]

Attachment: specialize NewGCThing to avoid integer mul/div in the allocator path

This patch specializes NewGCThing for JSObject, JSFunction, JSString and JSXML. We always allocate for sizeof(T), and don't try to save a few trailing words for cloned JSFunctions and certain JSXML objects.

Comment 3

[Andreas Gal :gal]

Comment on attachment 387767 [details] [diff] [review]
specialize NewGCThing to avoid integer mul/div in the allocator path

Brendan indicated his preference for adding back another size class for cloned Functions. I am a bit reluctant on that. I can't imagine it makes a big enough difference but I will go ahead and measure it.

Comment 4

[Andreas Gal :gal]

Attachment: patch

This addresses an offline-nit by Brendan to restore the previous behavior of not over-allocating cloned Functions. JSXML objects still potentially waste a few bytes, but W.

Comment 5

[Igor Bukanov]

(In reply to comment #4)
> Created an attachment (id=388141) [details]
> patch

What about passing flindex to NewGCThing with helper inline functions calculating the index statically? This will avoid the useless code duplication in jsgc.cpp.

Comment 6

[Andreas Gal :gal]

The path seems very sensitive to the generated code. I don't think there is a way to guess it. Do you want to sketch up a patch and we measure it?

Comment 7

[Andreas Gal :gal]

Btw, its not flindex thats the issue here. Its the individual THING_TO_FLAGP and FLAGP_TO_THING macro calculations, which result in simple shift if we specialize, but actual heavy-weight idiv/imul if we don't. Let me know if you want to do an alternative patch.

Comment 8

[Igor Bukanov]

(In reply to comment #7)
> Btw, its not flindex thats the issue here. Its the individual THING_TO_FLAGP
> and FLAGP_TO_THING macro calculations, which result in simple shift if we
> specialize, but actual heavy-weight idiv/imul if we don't. Let me know if you
> want to do an alternative patch.

You are right, it seems that the template function is a good way to optimize this.

Comment 9

[Andreas Gal :gal]

Attachment: patch

Comment 10

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/b0f849609c10

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/b0f849609c10