WIP: Introduce CompressibleString

third_party/WebKit/Source/wtf/text/CompressableString.cpp

Index: third_party/WebKit/Source/wtf/text/CompressableString.cpp
diff --git a/third_party/WebKit/Source/wtf/text/CompressableString.cpp b/third_party/WebKit/Source/wtf/text/CompressableString.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..e773778eaf4200fc579f2131307f505c968cc199
--- /dev/null
+++ b/third_party/WebKit/Source/wtf/text/CompressableString.cpp
@@ -0,0 +1,104 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "wtf/text/CompressableString.h"
+
+// TODO: This seems illegal: Fix checkdeps?
+#include "components/compression/compression_utils.h"

[esprehn, 2015/10/24 03:59:25]

you can't include from components inside blink, this also isn't planned for the
future either. If this is a general purpose compression lib it should probably
be in base.

In general I think this needs to be in platform though, and you need content to
implement an interface for doing the compression.

WTF is like our base, and base certainly can't depend on components/

[haraken, 2015/10/24 17:26:37]

Agreed. It is a bad idea to make wtf/ depend on components/. We can put
CompressableString in platform/.

[hajimehoshi, 2015/10/26 09:34:02]

Sure, I understand components is not available from wtf. I'll create an
interface for compression at platform.

+#include "wtf/Assertions.h"
+#include "wtf/Partitions.h"
+#include "wtf/text/WTFString.h"
+
+namespace WTF {
+
+PassRefPtr<CompressableString> CompressableString::create(const String& string)
+{
+    if (string.isNull())
+        return nullptr;
+    return adoptRef(new CompressableString(string));
+}
+
+unsigned CompressableString::length() const

[haraken, 2015/10/22 16:03:31]

Inline these simple functions.

[hajimehoshi, 2015/10/26 09:34:02]

Done.

+{
+    return m_originalLength;
+}
+
+bool CompressableString::isEmpty() const
+{
+    return m_originalLength == 0;
+}
+
+bool CompressableString::is8Bit() const
+{
+    return m_is8Bit;
+}
+
+void CompressableString::compress()

[haraken, 2015/10/22 16:03:31]

Your plan is to make CompressableString inherit from MemoryPurgeClient and call
compress() in MemoryPurgeClient::purgeMemory(), right?

I'm a bit concerned about the overhead of making all CompressableStrings
MemoryPurgeClient. Maybe it's better to create a HashSet<CompressableString*>
which keeps track of CompressableStrings larger than 50 KB (and make the wrapper
class of the HashSet MemoryPurgeClient).

  class LargeCompressableStrings : public MemoryPurgeClient {
    HashSet<CompressableString*> m_strings;
  };

[hajimehoshi, 2015/10/27 11:35:10]

Done. This is still not thread safe.

+{
+    ASSERT(m_impl);
+    ASSERT(!m_isCompressed);
+
+    std::string in, out;
+    if (m_is8Bit)
+        in = std::string(reinterpret_cast<const char*>(m_impl->characters8()), m_impl->sizeInBytes());

[esprehn, 2015/10/24 03:59:26]

this is making copies which is bad.

[hajimehoshi, 2015/10/26 09:34:03]

Yeah, the implementation is temporal. To fix this, I'd need to modify the
compression component, which accepts only std::string.

[hajimehoshi, 2015/10/27 11:35:10]

Avoiding copying requires hack to components/compression and it seems a little
tough work so far. I understand that using std::string causes unnecessary
copying but we can't avoid all copying. First, we need to allocate upper-bound
size which is a little bigger than input binary before compression and shrink
this after compression. 

This CL is for experiment to measure memory usage, so I'll return to this point
after that.

+    else
+        in = std::string(reinterpret_cast<const char*>(m_impl->characters16()), m_impl->sizeInBytes());

[esprehn, 2015/10/24 03:59:25]

more copies, also don't you want string16?

+    compression::GzipCompress(in, &out);
+
+    LChar* data = nullptr;
+    m_impl = StringImpl::createUninitialized(out.size() / sizeof(LChar), data);
+    memcpy(data, out.c_str(), out.size());

[esprehn, 2015/10/24 03:59:26]

Why are we storing the compressed data inside a StringImpl? It's not a valid
string anymore, you just want to store it in a buffer somewhere, not a
StringImpl. Maybe a SharedBuffer?

[hajimehoshi, 2015/10/27 09:43:56]

I agree to that StringImpl is not appropriate for this usage, but SharedBuffer
in platform is not available from wtf.

+
+    m_isCompressed = true;
+}
+
+String CompressableString::toString()
+{
+    ASSERT(m_impl);
+    if (isCompressed())

[haraken, 2015/10/22 16:03:31]

Add UNLIKELY.

[hajimehoshi, 2015/10/26 09:34:03]

Done.

+        uncompress();
+    return m_impl.get();
+}
+
+CompressableString::CompressableString()
+    : m_isCompressed(false)
+    , m_impl(StringImpl::empty())
+    , m_originalLength(0)
+    , m_is8Bit(m_impl->is8Bit())
+{
+}
+
+CompressableString::CompressableString(const String& string)
+    : m_isCompressed(false)
+    , m_impl(string.impl())
+    , m_originalLength(string.length())
+    , m_is8Bit(m_impl->is8Bit())
+{
+}
+
+void CompressableString::uncompress()
+{
+    ASSERT(m_isCompressed);
+    ASSERT(m_impl->is8Bit());
+
+    std::string in(reinterpret_cast<const char*>(m_impl->characters8()), m_impl->sizeInBytes());
+    std::string out;
+    compression::GzipUncompress(in, &out);
+
+    if (m_is8Bit) {
+        LChar* data = nullptr;
+        m_impl = StringImpl::createUninitialized(out.size() / sizeof(LChar), data);
+        memcpy(data, out.c_str(), out.size());
+    } else {
+        UChar* data = nullptr;
+        m_impl = StringImpl::createUninitialized(out.size() / sizeof(UChar), data);
+        memcpy(data, out.c_str(), out.size());
+    }
+
+    m_isCompressed = false;
+    ASSERT(m_originalLength == m_impl->length());
+}
+
+} // namespace WTF