smalloc: create separate module
authorTrevor Norris <trev.norris@gmail.com>
Fri, 12 Jul 2013 22:18:21 +0000 (15:18 -0700)
committerTrevor Norris <trev.norris@gmail.com>
Fri, 19 Jul 2013 20:36:13 +0000 (13:36 -0700)
It will be confusing if later on we add Buffer#dispose(), and smalloc is
its own cpp api anyways. So instead create a new require('smalloc') to
expose the previous Buffer.alloc/dispose methods, and expose copyOnto
and kMaxLength as well.

Other changes:
* Added documentation and additional tests.
* smalloc::CopyOnto has changed from using assert() to throwing errors
  on bad argument values because it is not exposed to the user.
* Minor style fixes.

doc/api/_toc.markdown
doc/api/all.markdown
doc/api/buffer.markdown
doc/api/smalloc.markdown [new file with mode: 0644]
lib/buffer.js
lib/smalloc.js [new file with mode: 0644]
node.gyp
src/smalloc.cc
test/simple/test-buffer.js
test/simple/test-smalloc.js

index fb27adb..8b0e24e 100644 (file)
@@ -24,6 +24,7 @@
 * [Query Strings](querystring.html)
 * [Readline](readline.html)
 * [REPL](repl.html)
+* [Smalloc](smalloc.html)
 * [Stream](stream.html)
 * [String Decoder](string_decoder.html)
 * [Timers](timers.html)
index e51e546..2a164ab 100644 (file)
@@ -34,3 +34,4 @@
 @include os
 @include debugger
 @include cluster
+@include smalloc
index e28c87e..a753aaa 100644 (file)
@@ -115,46 +115,6 @@ If totalLength is not provided, it is read from the buffers in the list.
 However, this adds an additional loop to the function, so it is faster
 to provide the length explicitly.
 
-### Class Method: Buffer.alloc(length, [receiver])
-
-* `length` Number
-* `receiver` Object, Optional, Default: `new Object`
-
-
-**(EXPERIMENTAL)** Returns object with allocated external array data.
-
-Buffers are backed by a simple allocator that only handles the assignation of
-external raw memory. This exposes that functionality.
-
-No pooling is performed for these allocations. So there's no form of memory
-leak.
-
-This can be used to create your own Buffer-like classes.
-
-    function SimpleData(n) {
-      this.length = n;
-      Buffer.alloc(this.length, this);
-    }
-
-    SimpleData.prototype = { /* ... */ };
-
-### Class Method: Buffer.dispose(obj)
-
-* `obj` Object
-
-
-**(EXPERIMENTAL)** Free memory that has been allocated to an object via
-`Buffer.alloc`.
-
-    var a = {};
-    Buffer.alloc(3, a);
-
-    // { '0': 0, '1': 0, '2': 0 }
-
-    Buffer.dispose(a);
-
-    // {}
-
 ### buf.length
 
 * Number
diff --git a/doc/api/smalloc.markdown b/doc/api/smalloc.markdown
new file mode 100644 (file)
index 0000000..abe4472
--- /dev/null
@@ -0,0 +1,101 @@
+# Smalloc
+
+    Stability: 1 - Experimental
+
+## smalloc.alloc(length[, receiver])
+
+* `length` Number `<= smalloc.kMaxLength`
+* `receiver` Object, Optional, Default: `new Object`
+
+Returns `receiver` with allocated external array data. If no `receiver` is
+passed then a new Object will be created and returned.
+
+Buffers are backed by a simple allocator that only handles the assignation of
+external raw memory. Smalloc exposes that functionality.
+
+This can be used to create your own Buffer-like classes. No other properties are
+set, so the user will need to keep track of other necessary information (e.g.
+`length` of the allocation).
+
+    function SimpleData(n) {
+      this.length = n;
+      smalloc.alloc(this.length, this);
+    }
+
+    SimpleData.prototype = { /* ... */ };
+
+It only checks if the `receiver` is an Object, and also not an Array. Because of
+this it is possible to allocate external array data to more than a plain Object.
+
+    function allocMe() { }
+    smalloc.alloc(3, allocMe);
+
+    // { [Function allocMe] '0': 0, '1': 0, '2': 0 }
+
+v8 does not support allocating external array data to an Array, and if passed
+will throw.
+
+## smalloc.copyOnto(source, sourceStart, dest, destStart, copyLength);
+
+* `source` Object with external array allocation
+* `sourceStart` Position to begin copying from
+* `dest` Object with external array allocation
+* `destStart` Position to begin copying onto
+* `copyLength` Length of copy
+
+Copy memory from one external array allocation to another. No arguments are
+optional, and any violation will throw.
+
+    var a = smalloc.alloc(4);
+    var b = smalloc.alloc(4);
+
+    for (var i = 0; i < 4; i++) {
+      a[i] = i;
+      b[i] = i * 2;
+    }
+
+    // { '0': 0, '1': 1, '2': 2, '3': 3 }
+    // { '0': 0, '1': 2, '2': 4, '3': 6 }
+
+    smalloc.copyOnto(b, 2, a, 0, 2);
+
+    // { '0': 4, '1': 6, '2': 2, '3': 3 }
+
+`copyOnto` automatically detects the length of the allocation internally, so no
+need to set any additional properties for this to work.
+
+## smalloc.dispose(obj)
+
+* `obj` Object
+
+Free memory that has been allocated to an object via `smalloc.alloc`.
+
+    var a = {};
+    smalloc.alloc(3, a);
+
+    // { '0': 0, '1': 0, '2': 0 }
+
+    smalloc.dispose(a);
+
+    // {}
+
+This is useful to reduce strain on the garbage collector, but developers must be
+careful. Cryptic errors may arise in applications that are difficult to trace.
+
+    var a = smalloc.alloc(4);
+    var b = smalloc.alloc(4);
+
+    // perform this somewhere along the line
+    smalloc.dispose(b);
+
+    // now trying to copy some data out
+    smalloc.copyOnto(b, 2, a, 0, 2);
+
+    // now results in:
+    // Error: source has no external array data
+
+`dispose()` does not support Buffers, and will throw if passed.
+
+## smalloc.kMaxLength
+
+Size of maximum allocation. This is also applicable to Buffer creation.
index a34d319..5592e31 100644 (file)
@@ -19,8 +19,8 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-var smalloc = process.binding('smalloc');
 var buffer = process.binding('buffer');
+var smalloc = process.binding('smalloc');
 var assert = require('assert');
 var util = require('util');
 var alloc = smalloc.alloc;
@@ -177,26 +177,6 @@ Buffer.concat = function(list, length) {
 };
 
 
-Buffer.alloc = function(n, obj) {
-  n = ~~n;
-  if (n < 0) n = 0;
-
-  if (n > kMaxLength)
-    throw new RangeError('n > kMaxLength');
-  if (Array.isArray(obj))
-    throw new TypeError('Arrays are not supported');
-
-  return alloc(obj != null ? obj : {}, n);
-};
-
-
-Buffer.dispose = function(obj) {
-  if (obj instanceof Buffer)
-    throw new TypeError('obj cannot be a Buffer');
-  smalloc.dispose(obj);
-};
-
-
 // pre-set for values that may exist in the future
 Buffer.prototype.length = undefined;
 Buffer.prototype.parent = undefined;
diff --git a/lib/smalloc.js b/lib/smalloc.js
new file mode 100644 (file)
index 0000000..b0226a0
--- /dev/null
@@ -0,0 +1,53 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var smalloc = process.binding('smalloc');
+var kMaxLength = smalloc.kMaxLength;
+
+exports.alloc = alloc;
+exports.copyOnto = smalloc.copyOnto;
+exports.dispose = dispose;
+
+// don't allow kMaxLength to accidentally be overwritten. it's a lot less
+// apparent when a primitive is accidentally changed.
+Object.defineProperty(exports, 'kMaxLength', {
+  writable: false,
+  value: kMaxLength
+});
+
+
+function alloc(n, obj) {
+  n = ~~n;
+
+  if (n > kMaxLength)
+    throw new RangeError('n > kMaxLength');
+  if (Array.isArray(obj))
+    throw new TypeError('Arrays are not supported');
+
+  return smalloc.alloc(obj === undefined ? {} : obj, n);
+}
+
+
+function dispose(obj) {
+  if (obj instanceof Buffer)
+    throw new TypeError('obj cannot be a Buffer');
+  smalloc.dispose(obj);
+}
index fb0286b..3809993 100644 (file)
--- a/node.gyp
+++ b/node.gyp
@@ -49,6 +49,7 @@
       'lib/querystring.js',
       'lib/readline.js',
       'lib/repl.js',
+      'lib/smalloc.js',
       'lib/stream.js',
       'lib/_stream_readable.js',
       'lib/_stream_writable.js',
index c3931fd..4b33c75 100644 (file)
@@ -21,6 +21,7 @@
 
 
 #include "node.h"
+#include "node_internals.h"
 #include "smalloc.h"
 
 #include "v8.h"
@@ -70,34 +71,45 @@ Cached<String> smalloc_sym;
 static bool using_alloc_cb;
 
 
-// for internal use: copyOnto(source, source_start, dest, dest_start, length)
+// copyOnto(source, source_start, dest, dest_start, copy_length)
 void CopyOnto(const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(node_isolate);
 
   Local<Object> source = args[0]->ToObject();
   Local<Object> dest = args[2]->ToObject();
+
+  if (!source->HasIndexedPropertiesInExternalArrayData())
+    return ThrowError("source has no external array data");
+  if (!dest->HasIndexedPropertiesInExternalArrayData())
+    return ThrowError("dest has no external array data");
+
   size_t source_start = args[1]->Uint32Value();
   size_t dest_start = args[3]->Uint32Value();
-  size_t length = args[4]->Uint32Value();
+  size_t copy_length = args[4]->Uint32Value();
   size_t source_length = source->GetIndexedPropertiesExternalArrayDataLength();
   size_t dest_length = dest->GetIndexedPropertiesExternalArrayDataLength();
   char* source_data = static_cast<char*>(
-                              source->GetIndexedPropertiesExternalArrayData());
+      source->GetIndexedPropertiesExternalArrayData());
   char* dest_data = static_cast<char*>(
-                                dest->GetIndexedPropertiesExternalArrayData());
+      dest->GetIndexedPropertiesExternalArrayData());
+
+  // necessary to check in case (source|dest)_start _and_ copy_length overflow
+  if (copy_length > source_length)
+    return ThrowRangeError("copy_length > source_length");
+  if (copy_length > dest_length)
+    return ThrowRangeError("copy_length > dest_length");
+  if (source_start > source_length)
+    return ThrowRangeError("source_start > source_length");
+  if (dest_start > dest_length)
+    return ThrowRangeError("dest_start > dest_length");
 
-  assert(source_data != NULL);
-  assert(dest_data != NULL);
-  // necessary to check in case (source|dest)_start _and_ length overflow
-  assert(length <= source_length);
-  assert(length <= dest_length);
-  assert(source_start <= source_length);
-  assert(dest_start <= dest_length);
   // now we can guarantee these will catch oob access and *_start overflow
-  assert(source_start + length <= source_length);
-  assert(dest_start + length <= dest_length);
+  if (source_start + copy_length > source_length)
+    return ThrowRangeError("source_start + copy_length > source_length");
+  if (dest_start + copy_length > dest_length)
+    return ThrowRangeError("dest_start + copy_length > dest_length");
 
-  memmove(dest_data + dest_start, source_data + source_start, length);
+  memmove(dest_data + dest_start, source_data + source_start, copy_length);
 }
 
 
@@ -129,9 +141,15 @@ void SliceOnto(const FunctionCallbackInfo<Value>& args) {
 void Alloc(const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(node_isolate);
 
+  if (!args[0]->IsObject())
+    return ThrowTypeError("argument must be an Object");
+
   Local<Object> obj = args[0]->ToObject();
   size_t length = args[1]->Uint32Value();
 
+  if (obj->HasIndexedPropertiesInExternalArrayData())
+    return ThrowTypeError("object already has external array data");
+
   Alloc(obj, length);
   args.GetReturnValue().Set(obj);
 }
@@ -151,6 +169,7 @@ void Alloc(Handle<Object> obj, size_t length) {
 
 
 void Alloc(Handle<Object> obj, char* data, size_t length) {
+  assert(!obj->HasIndexedPropertiesInExternalArrayData());
   Persistent<Object> p_obj(node_isolate, obj);
   node_isolate->AdjustAmountOfExternalAllocatedMemory(length);
   p_obj.MakeWeak(data, TargetCallback);
@@ -176,6 +195,7 @@ void TargetCallback(Isolate* isolate,
 }
 
 
+// for internal use: dispose(obj);
 void AllocDispose(const FunctionCallbackInfo<Value>& args) {
   AllocDispose(args[0]->ToObject());
 }
@@ -220,6 +240,8 @@ void Alloc(Handle<Object> obj,
            size_t length,
            FreeCallback fn,
            void* hint) {
+  assert(!obj->HasIndexedPropertiesInExternalArrayData());
+
   if (smalloc_sym.IsEmpty()) {
     smalloc_sym = String::New("_smalloc_p");
     using_alloc_cb = true;
index f186af9..2baf0d2 100644 (file)
@@ -950,39 +950,3 @@ assert.throws(function() {
   Buffer('', 'buffer');
 }, TypeError);
 
-
-// test Buffer alloc
-
-// arrays are unsupported by v8
-assert.throws(function() {
-  Buffer.alloc(0, []);
-}, TypeError);
-
-// can't create too large an alloc
-assert.throws(function() {
-  Buffer.alloc(0x3fffffff + 1);
-}, RangeError);
-
-// make sure values are assigned
-var b = {};
-Buffer.alloc(256, b);
-for (var i = 0; i < 256; i++)
-  b[i] = i;
-for (var i = 0; i < 256; i++)
-  assert.equal(b[i], i);
-assert.equal(b[257], undefined);
-
-// several other types that shouldn't throw
-Buffer.alloc(1, function() { });
-Buffer.alloc(1, /abc/);
-Buffer.alloc(1, new Date());
-
-
-// make sure disposal works
-var b = {};
-Buffer.alloc(5, b);
-for (var i = 0; i < 5; i++)
-  b[i] = i;
-Buffer.dispose(b);
-for (var i = 0; i < 5; i++)
-  assert.equal(b[i], undefined);
index 2a30621..3729adb 100644 (file)
 var common = require('../common');
 var assert = require('assert');
 
-var smalloc = process.binding('smalloc');
+// first grab js api's
+var smalloc = require('smalloc');
 var alloc = smalloc.alloc;
-var copyOnto = smalloc.copyOnto;
 var dispose = smalloc.dispose;
-var sliceOnto = smalloc.sliceOnto;
+var copyOnto = smalloc.copyOnto;
+var kMaxLength = smalloc.kMaxLength;
+// sliceOnto is volatile and cannot be exposed to users.
+var sliceOnto = process.binding('smalloc').sliceOnto;
 
 
-// verify initial allocation
+// verify allocation
 
-var b = alloc({}, 5);
+var b = alloc(5, {});
 assert.ok(typeof b === 'object');
 for (var i = 0; i < 5; i++)
   assert.ok(b[i] !== undefined);
 
 
 var b = {};
-var c = alloc(b, 5);
+var c = alloc(5, b);
 assert.equal(b, c);
 assert.deepEqual(b, c);
 
 
-var b = alloc({}, 5);
+var b = alloc(5, {});
 var c = {};
 c._data = sliceOnto(b, c, 0, 5);
 assert.ok(typeof c._data === 'object');
@@ -53,14 +56,14 @@ assert.deepEqual(b, c._data);
 
 // verify writes
 
-var b = alloc({}, 5);
+var b = alloc(5, {});
 for (var i = 0; i < 5; i++)
   b[i] = i;
 for (var i = 0; i < 5; i++)
   assert.equal(b[i], i);
 
 
-var b = alloc({}, 6);
+var b = alloc(6, {});
 var c0 = {};
 var c1 = {};
 c0._data = sliceOnto(b, c0, 0, 3);
@@ -75,9 +78,9 @@ for (var i = 3; i < 6; i++)
   assert.equal(b[i], i);
 
 
-var a = alloc({}, 6);
-var b = alloc({}, 6);
-var c = alloc({}, 12);
+var a = alloc(6, {});
+var b = alloc(6, {});
+var c = alloc(12, {});
 for (var i = 0; i < 6; i++) {
   a[i] = i;
   b[i] = i * 2;
@@ -90,9 +93,96 @@ for (var i = 0; i < 6; i++) {
 }
 
 
-// test disposal
+// verify alloc throws properly
+
+// arrays are not supported
+assert.throws(function() {
+  alloc(0, []);
+}, TypeError);
+
+
+// no allocations larger than kMaxLength
+assert.throws(function() {
+  alloc(kMaxLength + 1);
+}, RangeError);
+
+
+// no allocating on what's been allocated
+assert.throws(function() {
+  alloc(1, alloc(1));
+}, TypeError);
+
+
+// throw for values passed that are not objects
+assert.throws(function() {
+  alloc(1, 'a');
+}, TypeError);
+assert.throws(function() {
+  alloc(1, 1);
+}, TypeError);
+assert.throws(function() {
+  alloc(1, true);
+}, TypeError);
+
+
+// should not throw allocating to most objects
+alloc(1, function() { });
+alloc(1, /abc/);
+alloc(1, new Date());
+
+
+// very copyOnto throws properly
+
+// source must have data
+assert.throws(function() {
+  copyOnto({}, 0, alloc(1), 0, 0);
+}, Error);
+
 
-var b = alloc({}, 5);
+// dest must have data
+assert.throws(function() {
+  copyOnto(alloc(1), 0, {}, 0, 0);
+}, Error);
+
+
+// copyLength <= sourceLength
+assert.throws(function() {
+  copyOnto(alloc(1), 0, alloc(3), 0, 2);
+}, RangeError);
+
+
+// copyLength <= destLength
+assert.throws(function() {
+  copyOnto(alloc(3), 0, alloc(1), 0, 2);
+}, RangeError);
+
+
+// sourceStart <= sourceLength
+assert.throws(function() {
+  copyOnto(alloc(1), 3, alloc(1), 0, 1);
+}, RangeError);
+
+
+// destStart <= destLength
+assert.throws(function() {
+  copyOnto(alloc(1), 0, alloc(1), 3, 1);
+}, RangeError);
+
+
+// sourceStart + copyLength <= sourceLength
+assert.throws(function() {
+  copyOnto(alloc(3), 1, alloc(3), 0, 3);
+}, RangeError);
+
+
+// destStart + copyLength <= destLength
+assert.throws(function() {
+  copyOnto(alloc(3), 0, alloc(3), 1, 3);
+}, RangeError);
+
+
+// test disposal
+var b = alloc(5, {});
 dispose(b);
 for (var i = 0; i < 5; i++)
   assert.equal(b[i], undefined);