Permalink
Browse files

buffer: allocate memory with mmap()

Work around an issue with the glibc malloc() implementation where memory blocks
are never returned to the operating system when they are allocated with brk()
and have overlapping lifecycles.

Fixes #4283.
  • Loading branch information...
1 parent 01db736 commit 2433ec8276838e90136669d5b1215ba597f15fdd @bnoordhuis bnoordhuis committed Nov 18, 2012
Showing with 114 additions and 5 deletions.
  1. +2 −1 lib/buffer.js
  2. +72 −4 src/node_buffer.cc
  3. +3 −0 src/node_buffer.h
  4. +37 −0 test/simple/test-buffer-release.js
View
@@ -20,6 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.
var SlowBuffer = process.binding('buffer').SlowBuffer;
+var kPoolSize = process.binding('buffer').kPoolSize;
var assert = require('assert');
exports.INSPECT_MAX_BYTES = 50;
@@ -297,7 +298,7 @@ Buffer.isEncoding = function(encoding) {
-Buffer.poolSize = 8 * 1024;
+Buffer.poolSize = kPoolSize; // see src/node_buffer.h
var pool;
function allocPool() {
View
@@ -29,6 +29,12 @@
#include <assert.h>
#include <string.h> // memcpy
+#ifdef __POSIX__
+# include <sys/mman.h> // mmap
+# include <unistd.h> // sysconf
+# include <stdio.h> // perror
+#endif
+
#define MIN(a,b) ((a) < (b) ? (a) : (b))
#define BUFFER_CLASS_ID (0xBABE)
@@ -196,6 +202,67 @@ Buffer::~Buffer() {
}
+#if defined(__POSIX__)
+
+static unsigned int num_pool_buffers;
+static char* cached_pool_buffers[16];
+
+
+static inline void free_buf_mem(char* buf, size_t len) {
+ if (len == Buffer::kPoolSize &&
+ num_pool_buffers < ARRAY_SIZE(cached_pool_buffers)) {
+ cached_pool_buffers[num_pool_buffers++] = buf;
+ return;
+ }
+
+ if (munmap(buf, len)) {
+ perror("munmap");
+ abort();
+ }
+}
+
+
+static inline char* alloc_buf_mem(size_t len) {
+ size_t pagesize = sysconf(_SC_PAGESIZE);
+
+ len = ROUND_UP(len, pagesize);
+ if (len == Buffer::kPoolSize && num_pool_buffers > 0) {
+ return cached_pool_buffers[--num_pool_buffers];
+ }
+
+ int prot = PROT_READ | PROT_WRITE;
+ int flags = MAP_ANONYMOUS | MAP_PRIVATE;
+ char* buf = static_cast<char*>(mmap(NULL, len, prot, flags, -1, 0));
+
+ if (buf == NULL) {
+ TryCatch try_catch;
+ char errmsg[256];
+ snprintf(errmsg,
+ sizeof(errmsg),
+ "Out of memory, mmap(len=%lu) failed.",
+ static_cast<unsigned long>(len));
+ ThrowError(errmsg);
+ FatalException(try_catch);
+ abort();
+ }
+
+ return buf;
+}
+
+#else // !__POSIX__
+
+static inline void free_buf_mem(char* buf, size_t len) {
+ delete[] buf;
+}
+
+
+static inline char* alloc_buf_mem(size_t len) {
+ return new char[len];
+}
+
+#endif // __POSIX__
+
+
// if replace doesn't have a callback, data must be copied
// const_cast in Buffer::New requires this
void Buffer::Replace(char *data, size_t length,
@@ -205,7 +272,7 @@ void Buffer::Replace(char *data, size_t length,
if (callback_) {
callback_(data_, callback_hint_);
} else if (length_) {
- delete [] data_;
+ free_buf_mem(data_, length_);
V8::AdjustAmountOfExternalAllocatedMemory(
-static_cast<intptr_t>(sizeof(Buffer) + length_));
}
@@ -217,9 +284,8 @@ void Buffer::Replace(char *data, size_t length,
if (callback_) {
data_ = data;
} else if (length_) {
- data_ = new char[length_];
- if (data)
- memcpy(data_, data, length_);
+ data_ = alloc_buf_mem(length_);
+ if (data != NULL) memcpy(data_, data, length_);
V8::AdjustAmountOfExternalAllocatedMemory(sizeof(Buffer) + length_);
} else {
data_ = NULL;
@@ -830,6 +896,8 @@ void Buffer::Initialize(Handle<Object> target) {
Buffer::MakeFastBuffer);
target->Set(String::NewSymbol("SlowBuffer"), constructor_template->GetFunction());
+ target->Set(String::NewSymbol("kPoolSize"),
+ Integer::NewFromUnsigned(kPoolSize));
HeapProfiler::DefineWrapperClass(BUFFER_CLASS_ID, WrapperInfo);
}
View
@@ -68,6 +68,9 @@ class NODE_EXTERN Buffer: public ObjectWrap {
// mirrors deps/v8/src/objects.h
static const unsigned int kMaxLength = 0x3fffffff;
+ // exported in lib/buffer.js as Buffer.poolSize
+ static const unsigned int kPoolSize = 32768;
+
static v8::Persistent<v8::FunctionTemplate> constructor_template;
static bool HasInstance(v8::Handle<v8::Value> val);
@@ -0,0 +1,37 @@
+// 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 common = require('../common');
+var assert = require('assert');
+
+function alloc() {
+ var h = {};
+ for (var i = 0; i < 10000; ++i) h[i] = new Buffer(20000);
+ h = null;
+}
+
+alloc();
+alloc();
+alloc();
+
+// Note: this assertion fails when run under valgrind because valgrind
+// increases the RSS footprint of node with at least 50 MB.
+assert(process.memoryUsage().rss < 32*1024*1024);

3 comments on commit 2433ec8

@isaacs
isaacs commented on 2433ec8 Dec 16, 2012

This causes a huge performance hit on SmartOS, and doesn't compile on OS X.

The OS X issue is easily worked around: b527e69

But the performance issue is more concerning.

@isaacs
isaacs commented on 2433ec8 Dec 16, 2012

Actually, this causes a performance hit on Ubuntu, it looks like, though less pronounced.

@bnoordhuis
Member

Actually, this causes a performance hit on Ubuntu.

It's actually a little faster for me (with 32 kB buffers) on x86_64 3.7-rc. I posted some numbers here. What does uname -a print?

This causes a huge performance hit on SmartOS

Can you quantify that?

Please sign in to comment.