Revert "stream_base: WriteWrap::New/::Dispose"

This reverts commit fe36076.
nodejs · piscisaureus · Mar 13, 2015 · Mar 13, 2015 · Mar 13, 2015 · Mar 13, 2015
commit 995f4f4a8c76b41efc699f1f5fc58238037f00aa
diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
@@ -15,7 +15,6 @@ using v8::FunctionTemplate;
 using v8::Handle;
 using v8::HandleScope;
 using v8::Local;
-using v8::Object;
 using v8::PropertyAttribute;
 using v8::PropertyCallbackInfo;
 using v8::String;
@@ -83,31 +82,6 @@ void StreamBase::JSMethod(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set((wrap->*Method)(args));
 }
 
-
-WriteWrap* WriteWrap::New(Environment* env,
-                          Local<Object> obj,
-                          StreamBase* wrap,
-                          DoneCb cb,
-                          size_t extra) {
-  size_t storage_size = ROUND_UP(sizeof(WriteWrap), kAlignSize) + extra;
-  char* storage = new char[storage_size];
-
-  return new(storage) WriteWrap(env, obj, wrap, cb);
-}
-
-
-void WriteWrap::Dispose() {
-  this->~WriteWrap();
-  delete[] reinterpret_cast<char*>(this);
-}
-
-
-char* WriteWrap::Extra(size_t offset) {
-  return reinterpret_cast<char*>(this) +
-         ROUND_UP(sizeof(*this), kAlignSize) +
-         offset;
-}
-
 }  // namespace node
 
 #endif  // SRC_STREAM_BASE_INL_H_
diff --git a/src/stream_base.cc b/src/stream_base.cc
@@ -1,5 +1,4 @@
 #include "stream_base.h"
-#include "stream_base-inl.h"
 #include "stream_wrap.h"
 
 #include "node.h"
@@ -109,8 +108,6 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
   // Determine storage size first
   size_t storage_size = 0;
   for (size_t i = 0; i < count; i++) {
-    storage_size = ROUND_UP(storage_size, WriteWrap::kAlignSize);
-
     Handle<Value> chunk = chunks->Get(i * 2);
 
     if (Buffer::HasInstance(chunk))
@@ -127,7 +124,7 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
     else
       chunk_size = StringBytes::StorageSize(env->isolate(), string, encoding);
 
-    storage_size += chunk_size;
+    storage_size += chunk_size + 15;
   }
 
   if (storage_size > INT_MAX)
@@ -136,14 +133,13 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
   if (ARRAY_SIZE(bufs_) < count)
     bufs = new uv_buf_t[count];
 
-  WriteWrap* req_wrap = WriteWrap::New(env,
-                                       req_wrap_obj,
-                                       this,
-                                       AfterWrite,
-                                       storage_size);
+  storage_size += sizeof(WriteWrap);
+  char* storage = new char[storage_size];
+  WriteWrap* req_wrap =
+      new(storage) WriteWrap(env, req_wrap_obj, this, AfterWrite);
 
   uint32_t bytes = 0;
-  size_t offset = 0;
+  size_t offset = sizeof(WriteWrap);
   for (size_t i = 0; i < count; i++) {
     Handle<Value> chunk = chunks->Get(i * 2);
 
@@ -156,9 +152,9 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
     }
 
     // Write string
-    offset = ROUND_UP(offset, WriteWrap::kAlignSize);
+    offset = ROUND_UP(offset, 16);
     CHECK_LT(offset, storage_size);
-    char* str_storage = req_wrap->Extra(offset);
+    char* str_storage = storage + offset;
     size_t str_size = storage_size - offset;
 
     Handle<String> string = chunk->ToString(env->isolate());
@@ -191,8 +187,10 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
     ClearError();
   }
 
-  if (err)
-    req_wrap->Dispose();
+  if (err) {
+    req_wrap->~WriteWrap();
+    delete[] storage;
+  }
 
   return err;
 }
@@ -209,6 +207,7 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   const char* data = Buffer::Data(args[1]);
   size_t length = Buffer::Length(args[1]);
 
+  char* storage;
   WriteWrap* req_wrap;
   uv_buf_t buf;
   buf.base = const_cast<char*>(data);
@@ -225,14 +224,17 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   CHECK_EQ(count, 1);
 
   // Allocate, or write rest
-  req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite);
+  storage = new char[sizeof(WriteWrap)];
+  req_wrap = new(storage) WriteWrap(env, req_wrap_obj, this, AfterWrite);
 
   err = DoWrite(req_wrap, bufs, count, nullptr);
   req_wrap->Dispatched();
   req_wrap_obj->Set(env->async(), True(env->isolate()));
 
-  if (err)
-    req_wrap->Dispose();
+  if (err) {
+    req_wrap->~WriteWrap();
+    delete[] storage;
+  }
 
  done:
   const char* msg = Error();
@@ -273,13 +275,14 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
     return UV_ENOBUFS;
 
   // Try writing immediately if write size isn't too big
+  char* storage;
   WriteWrap* req_wrap;
   char* data;
   char stack_storage[16384];  // 16kb
   size_t data_size;
   uv_buf_t buf;
 
-  bool try_write = storage_size <= sizeof(stack_storage) &&
+  bool try_write = storage_size + 15 <= sizeof(stack_storage) &&
                    (!IsIPCPipe() || send_handle_obj.IsEmpty());
   if (try_write) {
     data_size = StringBytes::Write(env->isolate(),
@@ -305,9 +308,11 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
     CHECK_EQ(count, 1);
   }
 
-  req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size);
+  storage = new char[sizeof(WriteWrap) + storage_size + 15];
+  req_wrap = new(storage) WriteWrap(env, req_wrap_obj, this, AfterWrite);
 
-  data = req_wrap->Extra();
+  data = reinterpret_cast<char*>(ROUND_UP(
+      reinterpret_cast<uintptr_t>(storage) + sizeof(WriteWrap), 16));
 
   if (try_write) {
     // Copy partial data
@@ -350,8 +355,10 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
   req_wrap->Dispatched();
   req_wrap->object()->Set(env->async(), True(env->isolate()));
 
-  if (err)
-    req_wrap->Dispose();
+  if (err) {
+    req_wrap->~WriteWrap();
+    delete[] storage;
+  }
 
  done:
   const char* msg = Error();
@@ -397,7 +404,8 @@ void StreamBase::AfterWrite(WriteWrap* req_wrap, int status) {
   if (req_wrap->object()->Has(env->oncomplete_string()))
     req_wrap->MakeCallback(env->oncomplete_string(), ARRAY_SIZE(argv), argv);
 
-  req_wrap->Dispose();
+  req_wrap->~WriteWrap();
+  delete[] reinterpret_cast<char*>(req_wrap);
 }
 
 

diff --git a/src/stream_base.h b/src/stream_base.h
@@ -56,23 +56,6 @@ class ShutdownWrap : public ReqWrap<uv_shutdown_t>,
 class WriteWrap: public ReqWrap<uv_write_t>,
                  public StreamReq<WriteWrap> {
  public:
-  static inline WriteWrap* New(Environment* env,
-                               v8::Local<v8::Object> obj,
-                               StreamBase* wrap,
-                               DoneCb cb,
-                               size_t extra = 0);
-  inline void Dispose();
-  inline char* Extra(size_t offset = 0);
-
-  inline StreamBase* wrap() const { return wrap_; }
-
-  static void NewWriteWrap(const v8::FunctionCallbackInfo<v8::Value>& args) {
-    CHECK(args.IsConstructCall());
-  }
-
-  static const size_t kAlignSize = 16;
-
- protected:
   WriteWrap(Environment* env,
             v8::Local<v8::Object> obj,
             StreamBase* wrap,
@@ -83,16 +66,24 @@ class WriteWrap: public ReqWrap<uv_write_t>,
     Wrap(obj, this);
   }
 
-  void* operator new(size_t size) = delete;
   void* operator new(size_t size, char* storage) { return storage; }
 
   // This is just to keep the compiler happy. It should never be called, since
   // we don't use exceptions in node.
   void operator delete(void* ptr, char* storage) { UNREACHABLE(); }
 
+  inline StreamBase* wrap() const {
+    return wrap_;
+  }
+
+  static void NewWriteWrap(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    CHECK(args.IsConstructCall());
+  }
+
  private:
   // People should not be using the non-placement new and delete operator on a
   // WriteWrap. Ensure this never happens.
+  void* operator new(size_t size) { UNREACHABLE(); }
   void operator delete(void* ptr) { UNREACHABLE(); }
 
   StreamBase* const wrap_;

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -298,10 +298,11 @@ void TLSWrap::EncOut() {
 
   Local<Object> req_wrap_obj =
       env()->write_wrap_constructor_function()->NewInstance();
-  WriteWrap* write_req = WriteWrap::New(env(),
-                                        req_wrap_obj,
-                                        this,
-                                        EncOutCb);
+  char* storage = new char[sizeof(WriteWrap)];
+  WriteWrap* write_req = new(storage) WriteWrap(env(),
+                                                req_wrap_obj,
+                                                this,
+                                                EncOutCb);
 
   uv_buf_t buf[ARRAY_SIZE(data)];
   for (size_t i = 0; i < count; i++)
@@ -317,7 +318,8 @@ void TLSWrap::EncOut() {
 
 void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
   TLSWrap* wrap = req_wrap->wrap()->Cast<TLSWrap>();
-  req_wrap->Dispose();
+  req_wrap->~WriteWrap();
+  delete[] reinterpret_cast<char*>(req_wrap);
 
   // Handle error
   if (status) {