915482 - Gut gc/Barrier-inl.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

I have three patches that, when applied, leave gc/Barrier-inl.h with a mere three methods and 41 lines of code.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Move most of gc/Barrier-inl.h into gc/Barrier.h.

If you, as I was, are worried about the performance of EncapsulatedId::pre()
now that it contains a call -- via ShadowZoneOfObject() -- to the uninlined
ZoneOfObject(), fear not:  Cachegrind tells me that this patch makes us
execute 1.00003x *fewer* instructions on SunSpider.  Perhaps it's because
~EncapsulatedId() is now inlined when previously it wasn't.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Minimize gc/Barrier-inl.h includes.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Move some functions out of vm/Shape-inl.h, jsfuninlines.h and jsinferinlines.h.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Minimize vm/Shape-inl.h includes.

Comment 5

[Terrence Cole [:terrence]]

Comment on attachment 803445 [details] [diff] [review]
(part 1) - Move most of gc/Barrier-inl.h into gc/Barrier.h.

Review of attachment 803445 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. r=me

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 803450 [details] [diff] [review]
(part 2) - Minimize gc/Barrier-inl.h includes.

Review of attachment 803450 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 7

[Terrence Cole [:terrence]]

Comment on attachment 803451 [details] [diff] [review]
(part 3) - Move some functions out of vm/Shape-inl.h, jsfuninlines.h and jsinferinlines.h.

Review of attachment 803451 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: js/src/vm/Shape.h
@@ +979,5 @@
>                                  Shape ***pspp, bool adding = false);
>      static inline Shape *searchNoHashify(Shape *start, jsid id);
>  
> +#ifdef DEBUG
> +    bool preconditionForRemoveFromDictionary(ObjectImpl *obj);

Frankly, I'd be shocked -- and more than a little appalled -- if having dictionary mode shape management inlined is critical to our performance. I think you should just out-of-line all of the dictionary-mode shape management.

Comment 8

[Terrence Cole [:terrence]]

Comment on attachment 803452 [details] [diff] [review]
(part 4) - Minimize vm/Shape-inl.h includes.

Review of attachment 803452 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 9

[Nicholas Nethercote [inactive]]

Part 1:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a4bb2092618

Comment 10

[Nicholas Nethercote [inactive]]

Part 2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e621399eb90f

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5a4bb2092618

Comment 12

[Nicholas Nethercote [inactive]]

Part 3:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffe6e70b0183

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e621399eb90f

Comment 14

[Nicholas Nethercote [inactive]]

Part 4:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47e05e8df03b

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ffe6e70b0183
https://hg.mozilla.org/mozilla-central/rev/47e05e8df03b