637905 - Add infallibleAppend methods to Vector for use when the corresponding space has been reserve()d

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

Right now we JS_ALWAYS_TRUE them, but that's only going to trip in debug builds on OOM.  So you need methods specifically for this purpose, and the vector can internally tracked reserved size to perform the assertion in the methods.

I slightly changed some transplant methods in ways that seem correct, but Andreas should double-check those (in jsapi.cpp) just to be certain.  The rest is basically Luke-territory, or something -- the rest don't seem too domain-specific or otherwise worrisome to require more-focused knowledge of the code being changed.

This passes JS shell tests for me, will run it past a tinderbox just to be sure those are catching everything before pushing (to TM when it opens for post-4.0 stuff).

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Real patch, qref'd

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 516211 [details] [diff] [review]
Real patch, qref'd

Request which can be a follow-up: with infallibleAppend, you know that growStorageBy will succeed, so you can skip it.  This should shave off a couple always-inlined instructions; good if nothing else for binary size and i-cache locality.

>+        elts.infallibleAppend(elt); /* reserved above */

Could you remove this and all the other comments; its now implied by infallibleAppend and enforced by the mighty assert-hammer you added.

>+    /* Infallible variants usable when the corresponding space is reserved. */
>+    void infallibleAppend(const jschar c) { cb.infallibleAppend(c); }
>+    void infallibleAppend(const jschar *chars, size_t len) {
>+        cb.infallibleAppend(chars, len);
>+    }

Looks weird to me to have one all in line and the three given new lines.  Can you make the first match the others?

>+#ifdef DEBUG
>+    size_t mReserved;   /* Max elements of reserved or used space in this vector. */
>+#endif

Righteous idea.

Comment 3

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/30940051c457

I filed bug 639583 for the requested followup.

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/mozilla-central/rev/30940051c457