1451524 - DLL interceptors should share trampoline VM

Status: RESOLVED FIXED

(Core :: General, enhancement, P1)

Description

[(No longer employed by Mozilla) Aaron Klotz]

Currently the Windows DLL interceptor's memory usage is very inefficient.

Each instance independently allocates its own VM for trampoline space. By default, each instance of the interceptor commits one page of VM, despite the fact that each instance sets no more than a couple of hooks.

Furthermore, since Windows allocates VM on a 64KiB granularity, we lose an additional 12KiB of address space per instance due to fragmentation, which is especially bad for 32-bit builds.

Once bug 1432653 lands, we can easily swap out policies for VM allocation such that most interceptor instances should be able to share a single trampoline space.

The code is already written for this, I'm just splitting out this bug from the existing refactoring work.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Oh, and another bad thing about the current state: The trampoline space can't ever be freed; interceptors by design must leak their trampolines.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Aaron Klotz [:aklotz] from comment #0)
> Furthermore, since Windows allocates VM on a 64KiB granularity, we lose an
> additional 12KiB of address space per instance due to fragmentation, which
> is especially bad for 32-bit builds.

I think some of that beer I drank on my Mexico trip is still in my veins. Obviously we're losing 60KiB to fragmentation, not 12KiB. :-)

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add VM sharing policy for sharing trampoline space between interceptors

I don't trust MozReview to properly treat this patch independently from bug 1432653, so I'm using Splinter here...

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add VM sharing policy for sharing trampoline space between interceptors (r2)

Rebased atop latest revision of bug 1432653

Comment 5

[David Parks [:handyman]]

Comment on attachment 8968633 [details] [diff] [review]
Add VM sharing policy for sharing trampoline space between interceptors (r2)

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

lgtm

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27d5c7a1d4c112687b78b4a9f86857378bfc67bb
Bug 1451524: Add a VM policy to the DLL interceptor that allows multiple instances to share a single trampoline space; r=handyman

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/27d5c7a1d4c1

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Change the default interceptor VM policy from unique to shared

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Make interceptor shared VM policy compatible with changes from bug 1456054

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

I'm going to land the fixes to the shared policy but hold off on the default changeover until after the soft code freeze.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83d635a4720194bec17d98035878d33cca1aa0e8
Bug 1451524: Make interceptor shared VM policy compatible with changes from bug 1456054; r=handyman

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/83d635a47201

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e79baefa364046e0e4dd7c8610507de4605cd164

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/501e46865f4327f5e104d4db22d644e54effc30f
Bug 1451524: Switch the default interceptor VM policy over from unique to shared; r=handyman

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/501e46865f43