1271022 - add Vector::reallocToFit

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Luke Wagner [:luke]]

I noticed, while measuring memory use on the wasm demo that several MB were being waste on the excess capacity of Vectors living in the wasm::Module.  The classic C++ trick to clear capacity is copy+swap which one can write already in terms of Vector, but I'd like to avoid the copy, so I added a Vector::reallocToFit, which is only defined when IsPod<T> is true, and it uses realloc.

With these patches, I can see savings of 4.2MB (out of 14.0MB) in class(WasmModuleObject)/objects/malloc-heap/misc on http://webassembly.github.io/demo.

Comment 1

[Luke Wagner [:luke]]

Attachment: add-realloc-to-fit

Comment 2

[Luke Wagner [:luke]]

Attachment: use-realloc-to-fit

Comment 3

[Nicholas Nethercote [inactive]]

Comment on attachment 8749931 [details] [diff] [review]
add-realloc-to-fit

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

I'm wondering about the name. Should it have "pod" in it to indicate the restriction? Would "resize" be better than "realloc". So... maybe "podResizeToFit"? Not sure.

::: mfbt/Vector.h
@@ +233,5 @@
> +  static inline void
> +  reallocToFit(Vector<T, N, AP>& aV)
> +  {
> +    if (aV.usingInlineStorage() || aV.mLength == aV.mCapacity)
> +      return;

Nit: MFBT style is Gecko style which braces all if-blocks.

@@ +236,5 @@
> +    if (aV.usingInlineStorage() || aV.mLength == aV.mCapacity)
> +      return;
> +    T* newbuf = aV.template pod_realloc<T>(aV.mBegin, aV.mCapacity, aV.mLength);
> +    if (MOZ_UNLIKELY(!newbuf))
> +      return;

Ditto.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8749931 [details] [diff] [review]
add-realloc-to-fit

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

::: mfbt/Vector.h
@@ +573,5 @@
>  
>    /**
> +   * Calls the AllocPolicy's pod_realloc to release excess capacity. Since
> +   * realloc is only safe on PODs, this method fails to compile if IsPod<T>
> +   * is true.

nit: "if IsPod<T> is false" or "unless IsPod<T> is true"

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8749932 [details] [diff] [review]
use-realloc-to-fit

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

Nice savings!

Comment 6

[Luke Wagner [:luke]]

(In reply to Nicholas Nethercote [:njn] from comment #3)
Yes, that's a better name.  I had been thinking 'realloc' would imply PODness, but that's too subtle.

(In reply to Benjamin Bouvier [:bbouvier] from comment #4)
Oops, good catch!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29e3f72069b4
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f2ac4c6db27

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/29e3f72069b4
https://hg.mozilla.org/mozilla-central/rev/6f2ac4c6db27