postMessage(): transfer allocation costs along with value.

third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 107 matching lines @@
       byteLength, "SerializedScriptValue buffer")));
   m_dataBufferSize = byteLength;
   wireData.copyTo(reinterpret_cast<UChar*>(m_dataBuffer.get()), 0,
                   wireData.length());
 }
 
 SerializedScriptValue::~SerializedScriptValue() {
   // If the allocated memory was not registered before, then this class is
   // likely used in a context other than Worker's onmessage environment and the
   // presence of current v8 context is not guaranteed. Avoid calling v8 then.
-  if (m_externallyAllocatedMemory) {
+  if (m_externallyAllocatedMemory > 0) {
     ASSERT(v8::Isolate::GetCurrent());
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
         -m_externallyAllocatedMemory);
   }
 }
 
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::nullValue() {
   // UChar rather than uint8_t here to get host endian behavior.
   static const UChar kNullData[] = {0xff09, 0x3000};
   return create(reinterpret_cast<const char*>(kNullData), sizeof(kNullData));
@@ skipping 298 matching lines @@
 
       if (isNeuterable) {
         for (const auto& bufferHandle : bufferHandles)
           bufferHandle->Neuter();
       }
     }
   }
   return contents;
 }
 
+void SerializedScriptValue::unregisterMemoryAllocatedByCurrentScriptContext() {
+  if (m_externallyAllocatedMemory < 0)
+    return;
+  if (m_externallyAllocatedMemory) {
+    v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
+        -(m_externallyAllocatedMemory - 1));
+    m_externallyAllocatedMemory = 0;
+  }
+  // TODO: if other transferables start accounting for their external
+  // allocations with V8, extend this with corresponding cases.
+  if (m_arrayBufferContentsArray) {
+    for (size_t i = 0; i < m_arrayBufferContentsArray->size(); ++i) {
+      WTF::ArrayBufferContents& buffer = m_arrayBufferContentsArray->at(i);
+      buffer.adjustExternalAllocatedMemoryUponContextTransfer(
+          WTF::ArrayBufferContents::Leave);
+    }
+    // Assign a negative size to mark the value as needing re-registration
+    // of transferable allocation costs by its target context in
+    // registerMemoryAllocatedWithCurrentScriptContext().
+    m_externallyAllocatedMemory = -1;

[haraken, 2017/03/08 09:15:22]

I'd prefer preparing a dedicated boolean flag rather than hacking on
m_externallyAllocatedMemory.

[sof, 2017/03/08 09:40:19]

Given that all allocations are within base::kGenericMaxDirectMapped's range in
Blink, I don't think we need to pad out this object further for this one
ephemeral bit. ymmv.

[jbroman, 2017/03/08 16:56:46]

If you are concerned about the size of this object, could you not do, with
essentially the same semantics but fewer magic constants by a bitfield instead
of the sign bit. Something along the lines of:

uint64_t m_externallyAllocatedMemory : 63;                            // zero by
default
uint64_t m_externallyAllocatedMemoryForTransferrablesRegistered : 1;  // true by
default, IIUC?

void SerializedScriptValue::unregisterMemoryAllocatedByCurrentScriptContext() {
  if (m_externallyAllocatedMemory) {
    v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
        -static_cast<intptr_t>(m_externallyAllocatedMemory));
    m_externallyAllocatedMemory = 0;
  }

  // TODO: if other transferables start accounting for their external
  // allocations with V8, extend this with corresponding cases.
  if (!m_externallyAllocatedMemoryForTransferrablesRegistered)
    return;
  if (m_arrayBufferContentsArray) {
    for (const auto& contents : *m_arrayBufferContentsArray) {
      contents.adjustExternalAllocatedMemoryUponContextTransfer(
          WTF::ArrayBufferContents::Leave);
    }
  }
  m_externallyAllocatedMemoryForTransferrablesRegistered = false;
}

void SerializedScriptValue::registerMemoryAllocatedWithCurrentScriptContext() {
  if (!m_externallyAllocatedMemory) {
    m_externallyAllocatedMemory = dataLengthInBytes();
    v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
        static_cast<intptr_t>(m_externallyAllocatedMemory));
  }

  // TODO: if other transferables start accounting for their external
  // allocations with V8, extend this with corresponding cases.
  if (m_externallyAllocatedMemoryForTransferrablesRegistered)
    return;

  if (m_arrayBufferContentsArray) {
    for (const auto& contents : *m_arrayBufferContentsArray) {
      contents.adjustExternalAllocatedMemoryUponContextTransfer(
          WTF::ArrayBufferContents::Enter);
    }
  }
  m_externallyAllocatedMemoryForTransferrablesRegistered = true;
}

To me, this is easier to reason about.

There's a case to be made that m_externallyAllocatedMemory could itself be
replaced by a boolean (since after serialization it's either equal to
dataLengthInBytes or it's zero, which is only two states).

+  }
+}
+
 void SerializedScriptValue::registerMemoryAllocatedWithCurrentScriptContext() {
-  if (m_externallyAllocatedMemory)
+  if (m_externallyAllocatedMemory > 0)
     return;
 
-  m_externallyAllocatedMemory = static_cast<intptr_t>(dataLengthInBytes());
+  bool registerTransferables = m_externallyAllocatedMemory < 0;
+  m_externallyAllocatedMemory = static_cast<int64_t>(dataLengthInBytes());
+  DCHECK_GE(m_externallyAllocatedMemory, 0);
   v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
       m_externallyAllocatedMemory);
+  if (registerTransferables) {
+    DCHECK(m_arrayBufferContentsArray);
+    for (size_t i = 0; i < m_arrayBufferContentsArray->size(); ++i) {
+      WTF::ArrayBufferContents& buffer = m_arrayBufferContentsArray->at(i);
+      buffer.adjustExternalAllocatedMemoryUponContextTransfer(
+          WTF::ArrayBufferContents::Enter);
+    }
+  }
 }
 
 }  // namespace blink