503669 - TM: Make js_ValueToStringId inline using jsatom-inl.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

In a microbenchmark that goes in a circle and sets the same property over and over, we spend about 2.5% of our time entering and leaving js_ValueToStringId, which has a quick fast path otherwise.

This patch makes it inline, but to solve the dependency hell (jsatom + jsstr), I had to push the inlines into a separate file. I would like to do this for a bunch of other things. What do people think?

Comment 1

[Andreas Gal :gal]

Attachment: patch

Asking for Brendan's review. This changes the source structure somewhat.

Comment 2

[Andreas Gal :gal]

Attachment: and the right patch this time

Comment 3

[Brendan Eich [:brendan]]

Suggest a standard (until we rename all files) other than -inl suffixing, based on existing convention: jsfooinlines.h for jsfoo.h's inlines that need to include as if they were .cpp files. Long-ish but it gets the point across without ugly -cybercrud.

/be

Comment 4

[Andreas Gal :gal]

Attachment: patch

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 388303 [details] [diff] [review]
patch

>diff --git a/js/src/jsatominlines.h b/js/src/jsatominlines.h

>+JS_BEGIN_EXTERN_C
>+JS_END_EXTERN_C

I don't think these are necessary, certainly not for SpiderMonkey-internal methods.  Other than that all cool.

Comment 6

[Andreas Gal :gal]

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

Comment 7

[Robert Sayre]

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