Merge pull request #1225 from svaarala/autospawn-typedarray-buffer-pr…

…operty Spawn typed array .buffer ArrayBuffer lazily on first access
svaarala · Jan 2, 2017 · 8816d05 · 8816d05
2 parents 442fa27 + 0381ade
commit 8816d05
Show file tree

Hide file tree

Showing 8 changed files with 109 additions and 73 deletions.
diff --git a/RELEASES.rst b/RELEASES.rst
@@ -2369,5 +2369,9 @@ Planned
 2.1.0 (XXXX-XX-XX)
 ------------------
 
+* Spawn the ArrayBuffer object backing a typed array lazily when its .buffer
+  property is first read, reducing memory usage in common cases where the view
+  is constructed directly without needing the ArrayBuffer object (GH-1225)
+
 3.0.0 (XXXX-XX-XX)
 ------------------
diff --git a/src-input/duk_api_public.h.in b/src-input/duk_api_public.h.in
@@ -520,19 +520,18 @@ DUK_EXTERNAL_DECL void *duk_push_buffer_raw(duk_context *ctx, duk_size_t size, d
 #define duk_push_external_buffer(ctx) \
 	((void) duk_push_buffer_raw((ctx), 0, DUK_BUF_FLAG_DYNAMIC | DUK_BUF_FLAG_EXTERNAL))
 
-#define DUK_BUFOBJ_CREATE_ARRBUF       (1 << 4)  /* internal flag: create backing ArrayBuffer; keep in one byte */
 #define DUK_BUFOBJ_ARRAYBUFFER         0
-#define DUK_BUFOBJ_NODEJS_BUFFER       (1 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_DATAVIEW            (2 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_INT8ARRAY           (3 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_UINT8ARRAY          (4 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_UINT8CLAMPEDARRAY   (5 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_INT16ARRAY          (6 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_UINT16ARRAY         (7 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_INT32ARRAY          (8 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_UINT32ARRAY         (9 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_FLOAT32ARRAY        (10 | DUK_BUFOBJ_CREATE_ARRBUF)
-#define DUK_BUFOBJ_FLOAT64ARRAY        (11 | DUK_BUFOBJ_CREATE_ARRBUF)
+#define DUK_BUFOBJ_NODEJS_BUFFER       1
+#define DUK_BUFOBJ_DATAVIEW            2
+#define DUK_BUFOBJ_INT8ARRAY           3
+#define DUK_BUFOBJ_UINT8ARRAY          4
+#define DUK_BUFOBJ_UINT8CLAMPEDARRAY   5
+#define DUK_BUFOBJ_INT16ARRAY          6
+#define DUK_BUFOBJ_UINT16ARRAY         7
+#define DUK_BUFOBJ_INT32ARRAY          8
+#define DUK_BUFOBJ_UINT32ARRAY         9
+#define DUK_BUFOBJ_FLOAT32ARRAY        10
+#define DUK_BUFOBJ_FLOAT64ARRAY        11
 
 DUK_EXTERNAL_DECL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer, duk_size_t byte_offset, duk_size_t byte_length, duk_uint_t flags);
 

diff --git a/src-input/duk_api_stack.c b/src-input/duk_api_stack.c
@@ -4377,7 +4377,7 @@ DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer,
 	DUK_ASSERT(uint_added >= uint_offset && uint_added >= uint_length);
 
 	DUK_ASSERT_DISABLE(flags >= 0);  /* flags is unsigned */
-	lookupidx = flags & 0x0f;  /* 4 low bits */
+	lookupidx = flags;
 	if (lookupidx >= sizeof(duk__bufobj_flags_lookup) / sizeof(duk_uint32_t)) {
 		goto arg_error;
 	}
@@ -4407,39 +4407,9 @@ DUK_EXTERNAL void duk_push_buffer_object(duk_context *ctx, duk_idx_t idx_buffer,
 	/* TypedArray views need an automatic ArrayBuffer which must be
 	 * provided as .buffer property of the view.  The ArrayBuffer is
 	 * referenced via duk_hbufobj->buf_prop and an inherited .buffer
-	 * accessor returns it.
-	 *
-	 * The ArrayBuffer offset is always set to zero, so that if one
-	 * accesses the ArrayBuffer at the view's .byteOffset, the value
-	 * matches the view at index 0.
+	 * accessor returns it.  The ArrayBuffer is created lazily on first
+	 * access so we don't need to do anything more here.
 	 */
-	if (flags & DUK_BUFOBJ_CREATE_ARRBUF) {
-		duk_hbufobj *h_arrbuf;
-
-		h_arrbuf = duk_push_bufobj_raw(ctx,
-		                               DUK_HOBJECT_FLAG_EXTENSIBLE |
-		                               DUK_HOBJECT_FLAG_BUFOBJ |
-		                               DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_ARRAYBUFFER),
-		                               DUK_BIDX_ARRAYBUFFER_PROTOTYPE);
-		DUK_ASSERT(h_arrbuf != NULL);
-
-		h_arrbuf->buf = h_val;
-		DUK_HBUFFER_INCREF(thr, h_val);
-		h_arrbuf->offset = 0;
-		h_arrbuf->length = uint_offset + uint_length;  /* Wrap checked above. */
-		DUK_ASSERT(h_arrbuf->shift == 0);
-		h_arrbuf->elem_type = DUK_HBUFOBJ_ELEM_UINT8;
-		DUK_ASSERT(h_arrbuf->is_typedarray == 0);
-		DUK_ASSERT_HBUFOBJ_VALID(h_arrbuf);
-		DUK_ASSERT(h_arrbuf->buf_prop == NULL);
-
-		DUK_ASSERT(h_bufobj->buf_prop == NULL);
-		h_bufobj->buf_prop = (duk_hobject *) h_arrbuf;
-		DUK_HBUFOBJ_INCREF(thr, h_arrbuf);  /* Now reachable and accounted for. */
-
-		duk_pop(ctx);
-	}
-
 	return;
 
  range_error:

diff --git a/src-input/duk_bi_buffer.c b/src-input/duk_bi_buffer.c
@@ -721,7 +721,6 @@ DUK_INTERNAL duk_ret_t duk_bi_typedarray_constructor(duk_context *ctx) {
 	duk_tval *tv;
 	duk_hobject *h_obj;
 	duk_hbufobj *h_bufobj = NULL;
-	duk_hbufobj *h_bufarr = NULL;
 	duk_hbufobj *h_bufarg = NULL;
 	duk_hbuffer *h_val;
 	duk_small_uint_t magic;
@@ -934,15 +933,17 @@ DUK_INTERNAL duk_ret_t duk_bi_typedarray_constructor(duk_context *ctx) {
 
 	/* ArrayBuffer argument is handled specially above; the rest of the
 	 * argument variants are handled by shared code below.
+	 *
+	 * ArrayBuffer in h_bufobj->buf_prop is intentionally left unset.
+	 * It will be automatically created by the .buffer accessor on
+	 * first access.
 	 */
 
-	/* Push a new ArrayBuffer (becomes view .buffer) */
-	h_bufarr = duk__push_arraybuffer_with_length(ctx, byte_length);
-	DUK_ASSERT(h_bufarr != NULL);
-	h_val = h_bufarr->buf;
+	/* Push the resulting view object on top of a plain fixed buffer. */
+	(void) duk_push_fixed_buffer(ctx, byte_length);
+	h_val = duk_known_hbuffer(ctx, -1);
 	DUK_ASSERT(h_val != NULL);
 
-	/* Push the resulting view object and attach the ArrayBuffer. */
 	h_bufobj = duk_push_bufobj_raw(ctx,
 	                               DUK_HOBJECT_FLAG_EXTENSIBLE |
 	                               DUK_HOBJECT_FLAG_BUFOBJ |
@@ -958,12 +959,6 @@ DUK_INTERNAL duk_ret_t duk_bi_typedarray_constructor(duk_context *ctx) {
 	h_bufobj->is_typedarray = 1;
 	DUK_ASSERT_HBUFOBJ_VALID(h_bufobj);
 
-	/* Set .buffer */
-	DUK_ASSERT(h_bufobj->buf_prop == NULL);
-	h_bufobj->buf_prop = (duk_hobject *) h_bufarr;
-	DUK_ASSERT(h_bufarr != NULL);
-	DUK_HBUFOBJ_INCREF(thr, h_bufarr);
-
 	/* Copy values, the copy method depends on the arguments.
 	 *
 	 * Copy mode decision may depend on the validity of the underlying
@@ -2878,30 +2873,63 @@ DUK_INTERNAL duk_ret_t duk_bi_buffer_writefield(duk_context *ctx) {
  */
 
 #if defined(DUK_USE_BUFFEROBJECT_SUPPORT)
+DUK_LOCAL duk_hbufobj *duk__autospawn_arraybuffer(duk_context *ctx, duk_hbuffer *h_buf) {
+	duk_hbufobj *h_res;
+
+	h_res = duk_push_bufobj_raw(ctx,
+	                            DUK_HOBJECT_FLAG_EXTENSIBLE |
+	                            DUK_HOBJECT_FLAG_BUFOBJ |
+	                            DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_ARRAYBUFFER),
+	                            DUK_BIDX_ARRAYBUFFER_PROTOTYPE);
+	DUK_ASSERT(h_res != NULL);
+	DUK_UNREF(h_res);
+
+	duk__set_bufobj_buffer(ctx, h_res, h_buf);
+	DUK_ASSERT_HBUFOBJ_VALID(h_res);
+	DUK_ASSERT(h_res->buf_prop == NULL);
+	return h_res;
+}
+
 DUK_INTERNAL duk_ret_t duk_bi_typedarray_buffer_getter(duk_context *ctx) {
 	duk_hbufobj *h_bufobj;
 
 	h_bufobj = (duk_hbufobj *) duk__getrequire_bufobj_this(ctx, DUK__BUFOBJ_FLAG_THROW /*flags*/);
 	DUK_ASSERT(h_bufobj != NULL);
 	if (DUK_HEAPHDR_IS_BUFFER((duk_heaphdr *) h_bufobj)) {
-		duk_hbufobj *h_res;
-		duk_hbuffer *h_buf;
-
-		h_buf = (duk_hbuffer *) h_bufobj;
-		h_res = duk_push_bufobj_raw(ctx,
-		                            DUK_HOBJECT_FLAG_EXTENSIBLE |
-		                            DUK_HOBJECT_FLAG_BUFOBJ |
-		                            DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_ARRAYBUFFER),
-		                            DUK_BIDX_ARRAYBUFFER_PROTOTYPE);
-		DUK_ASSERT(h_res != NULL);
-		DUK_UNREF(h_res);
-
-		duk__set_bufobj_buffer(ctx, h_res, h_buf);
-		DUK_ASSERT_HBUFOBJ_VALID(h_res);
-
-		DUK_DD(DUK_DDPRINT("autospawned .buffer ArrayBuffer: %!iT", duk_get_tval(ctx, -1)));
+		DUK_DD(DUK_DDPRINT("autospawn ArrayBuffer for plain buffer"));
+		(void) duk__autospawn_arraybuffer(ctx, (duk_hbuffer *) h_bufobj);
 		return 1;
 	} else {
+		if (h_bufobj->buf_prop == NULL &&
+		    DUK_HOBJECT_GET_CLASS_NUMBER((duk_hobject *) h_bufobj) != DUK_HOBJECT_CLASS_ARRAYBUFFER &&
+		    h_bufobj->buf != NULL) {
+			duk_hbufobj *h_arrbuf;
+
+			DUK_DD(DUK_DDPRINT("autospawn ArrayBuffer for typed array or DataView"));
+			h_arrbuf = duk__autospawn_arraybuffer(ctx, h_bufobj->buf);
+
+			if (h_bufobj->buf_prop == NULL) {
+				/* Must recheck buf_prop, in case ArrayBuffer
+				 * alloc had a side effect which already filled
+				 * it!
+				 */
+
+				/* Set ArrayBuffer's .byteOffset and .byteLength based
+				 * on the view so that Arraybuffer[view.byteOffset]
+				 * matches view[0].
+				 */
+				h_arrbuf->offset = 0;
+				DUK_ASSERT(h_bufobj->offset + h_bufobj->length >= h_bufobj->offset);  /* Wrap check on creation. */
+				h_arrbuf->length = h_bufobj->offset + h_bufobj->length;
+				DUK_ASSERT(h_arrbuf->buf_prop == NULL);
+
+				DUK_ASSERT(h_bufobj->buf_prop == NULL);
+				h_bufobj->buf_prop = (duk_hobject *) h_arrbuf;
+				DUK_HBUFOBJ_INCREF(thr, h_arrbuf);  /* Now reachable and accounted for. */
+			}
+
+			/* Left on stack; pushed for the second time below (OK). */
+		}
 		if (h_bufobj->buf_prop) {
 			duk_push_hobject(ctx, h_bufobj->buf_prop);
 			return 1;

diff --git a/tests/memory/test-function-expression-1.js b/tests/memory/test-function-expression-1.js
@@ -0,0 +1,8 @@
+function test() {
+    var arr = [];
+    while (arr.length < 1e5) {
+        arr.push(function () {});
+    }
+    print(arr.length + ' anonymous functions created');
+}
+test();
diff --git a/tests/memory/test-function-expression-2.js b/tests/memory/test-function-expression-2.js
@@ -0,0 +1,11 @@
+function test() {
+    var arr = [];
+    var fn;
+    while (arr.length < 1e4) {
+        fn = function () {};
+        fn.prototype = null;
+        arr.push(fn);
+    }
+    print(arr.length + ' anonymous functions created');
+}
+test();
diff --git a/tests/memory/test-plain-buffer-1.js b/tests/memory/test-plain-buffer-1.js
@@ -0,0 +1,8 @@
+function test() {
+    var arr = [];
+    while (arr.length < 1e5) {
+        arr.push(Uint8Array.allocPlain(256));
+    }
+    print(arr.length + ' plain buffers created');
+}
+test();
diff --git a/tests/memory/test-uint8array-1.js b/tests/memory/test-uint8array-1.js
@@ -0,0 +1,8 @@
+function test() {
+    var arr = [];
+    while (arr.length < 1e5) {
+        arr.push(new Uint8Array(256));
+    }
+    print(arr.length + ' Uint8Arrays created');
+}
+test();