This repository has been archived by the owner. It is now read-only.

Buffer momery pool was so inefficent... #2035

Closed
windyrobin opened this Issue Nov 7, 2011 · 2 comments

Comments

Projects
None yet
3 participants
@windyrobin

windyrobin commented Nov 7, 2011

@ry
@bnoordhuis

the issue 2020 inspires me to have a deep thinking on the this issue,
I'm really sorry for these complains ,but I'm in favor of node,
and I really expect it become better and better ..

User

For the application users , the default Buffer pool size is 8*1024 and
many Buffer share the same SlowBuffer instatnce:

lib/buffer.js

this.parent = pool;
this.offset = pool.used;
pool.used += this.length;

so only when all Buffers that refer to the same SlowBuffer instance are freed ,the SlowBuffer is freed,
even only a buffer whose length is 1 byte could cause the 8*1024 block persist...

Stream(net/file)

the stream read-buffer-size is 64*1024 ,the code writen in

uv/src/unix.stream.c :

static void uv__read(uv_stream_t* stream) {
...
 buf = stream->alloc_cb((uv_handle_t*)stream, 64 * 1024);
...

we use the slab which size is 1024_1024
_steam_wrap.cc* :

if (slab_v.IsEmpty()) {
     // No slab currently. Create a new one.
    slab = NewSlab(global, wrap->object_);
  } else {
     // Use existing slab.
     Local<Object> slab_obj = slab_v->ToObject();
     slab = Buffer::Data(slab_obj);
     assert(Buffer::Length(slab_obj) == SLAB_SIZE);
     assert(SLAB_SIZE >= slab_used);

     // If less than 64kb is remaining on the slab allocate a new one.
     if (SLAB_SIZE - slab_used < 64 * 1024) {
       slab = NewSlab(global, wrap->object_);
     } else {
       wrap->object_->SetHiddenValue(slab_sym, slab_obj);
     }
   }

  uv_buf_t buf;
  buf.base = slab + slab_used;
  buf.len = MIN(SLAB_SIZE - slab_used, suggested_size);

  wrap->slab_offset_ = slab_used;
  slab_used += buf.len;

  handle_that_last_alloced = reinterpret_cast<uv_stream_t*>(handle);
  return buf;

we all know that 64K is really large for most scenarios ,and the 'nread' is usually very small,
but we only subtract the used size when :

if (handle_that_last_alloced == handle) {
     slab_used -= (buf.len - nread);
 }

this's really a big waste!!

Allocator

we don't reuse the memory block ,we just new/delete the array :
src/buffer.cc

void Buffer::Replace(char *data, size_t length,
                     free_callback callback, void *hint) {
...
delete [] data_;
...
data_ = new char[length_];

I think we should have a deep consideration on memory usage ,
if we had a nice and efficient buffer-memory-pool.
we could get a great lift in socket/file operation performance ...

@tereska

This comment has been minimized.

Show comment
Hide comment
@tereska

tereska commented Nov 16, 2012

+1

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Nov 17, 2012

Member

Closing, we use an improved slab allocator these days.

Member

bnoordhuis commented Nov 17, 2012

Closing, we use an improved slab allocator these days.

@bnoordhuis bnoordhuis closed this Nov 17, 2012

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.