Refactor string_output_buffer to support "Reserve".
On its own, it is 2% faster, but not statistically significant.
The main benefit here is that if we support reserve, we can implement
something akin to the following. Then we can, instead of allocating a
buffer for the escaped value, generating it, then copying it to the
buffer, write it directly.
struct NinjaEscaped {
inner: &str
}
impl WriteBuf for NinjaEscaped {
fn MaximumSize(&self) -> usize { inner.size() * 3 }
fn Write(&self, dst: &mut [char], upto: &mut idx) {
for c in self.inner.chars() {
if needs_ninja_escape(c) {
dst[upto++] = '\'
}
dst[upto++] = c
}
}
Change-Id: I82469ade090787853e9c83085c7c1dfa6a6a6964
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/27640
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/string_output_buffer.cc b/src/gn/string_output_buffer.cc
index 2435fa1..d02f707 100644
--- a/src/gn/string_output_buffer.cc
+++ b/src/gn/string_output_buffer.cc
@@ -6,58 +6,50 @@
#include <fstream>
+#include "base/compiler_specific.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "gn/err.h"
#include "gn/file_writer.h"
#include "gn/filesystem_utils.h"
+void StringOutputBuffer::Reserve(size_t n) {
+ if (UNLIKELY(page_free_size() < n)) {
+ if (rw_.size > 0) {
+ ro_.push_back(std::move(rw_));
+ ro_size_ += rw_.size;
+ }
+
+ // Pick the minimum number of pages required to store n bytes.
+ // So technically, our "Page" could actually be multiple pages
+ // But unless you attempt to call append on a ridiculously large string,
+ // this should never happen in practice.
+ rw_capacity_ = (n + kPageSize - 1) & ~(kPageSize - 1);
+ rw_ = Page{
+ .buf = std::make_unique<char[]>(rw_capacity_),
+ .size = 0,
+ };
+ }
+}
+
std::string StringOutputBuffer::str() const {
std::string result;
- size_t data_size = size();
- result.reserve(data_size);
- for (size_t nn = 0; nn < pages_.size(); ++nn) {
- size_t wanted_size = std::min(kPageSize, data_size - nn * kPageSize);
- result.append(pages_[nn]->data(), wanted_size);
+ result.reserve(size());
+ for (const auto& page : ro_) {
+ result.append(page.buf.get(), page.size);
+ }
+ if (rw_.size > 0) {
+ result.append(rw_.buf.get(), rw_.size);
}
return result;
}
-void StringOutputBuffer::Append(const char* str, size_t len) {
- Append(std::string_view(str, len));
-}
-
-void StringOutputBuffer::Append(std::string_view str) {
- while (str.size() > 0) {
- if (page_free_size() == 0) {
- // Allocate a new page.
- pages_.push_back(std::make_unique<Page>());
- pos_ = 0;
- }
- size_t size = std::min(page_free_size(), str.size());
- memcpy(pages_.back()->data() + pos_, str.data(), size);
- pos_ += size;
- str.remove_prefix(size);
- }
-}
-
-void StringOutputBuffer::Append(char c) {
- if (page_free_size() == 0) {
- // Allocate a new page.
- pages_.push_back(std::make_unique<Page>());
- pos_ = 0;
- }
- pages_.back()->data()[pos_] = c;
- pos_ += 1;
-}
-
bool StringOutputBuffer::ContentsEqual(const base::FilePath& file_path) const {
// Compare file and stream sizes first. Quick and will save us some time if
// they are different sizes.
- size_t data_size = size();
int64_t file_size;
if (!base::GetFileSize(file_path, &file_size) ||
- static_cast<size_t>(file_size) != data_size) {
+ static_cast<size_t>(file_size) != size()) {
return false;
}
@@ -66,18 +58,26 @@
if (!file.is_open())
return false;
- size_t page_count = pages_.size();
- Page file_page;
- for (size_t nn = 0; nn < page_count; ++nn) {
- size_t wanted_size = std::min(data_size - nn * kPageSize, kPageSize);
- file.read(file_page.data(), wanted_size);
+ std::vector<char> file_page;
+ file_page.resize(kPageSize);
+ auto check_page = [&](const Page& page) {
+ // Don't use reserve because if we do that, capacity is high enough but not
+ // size.
+ if (UNLIKELY(file_page.size() < page.size)) {
+ file_page.resize(page.size);
+ }
+ file.read(file_page.data(), page.size);
if (!file.good())
return false;
- if (memcmp(file_page.data(), pages_[nn]->data(), wanted_size) != 0)
+ return memcmp(file_page.data(), page.buf.get(), page.size) == 0;
+ };
+
+ for (const auto& page : ro_) {
+ if (!check_page(page))
return false;
}
- return true;
+ return rw_.size == 0 || check_page(rw_);
}
// Write the contents of this instance to a file at |file_path|.
@@ -93,18 +93,17 @@
return false;
}
- size_t data_size = size();
- size_t page_count = pages_.size();
-
FileWriter writer;
bool success = writer.Create(file_path);
if (success) {
- for (size_t nn = 0; nn < page_count; ++nn) {
- size_t wanted_size = std::min(data_size - nn * kPageSize, kPageSize);
- success = writer.Write(std::string_view(pages_[nn]->data(), wanted_size));
+ for (const auto& page : ro_) {
+ success = writer.Write(std::string_view(page.buf.get(), page.size));
if (!success)
break;
}
+ if (success && rw_.size > 0) {
+ success = writer.Write(std::string_view(rw_.buf.get(), rw_.size));
+ }
}
if (!writer.Close())
success = false;
diff --git a/src/gn/string_output_buffer.h b/src/gn/string_output_buffer.h
index 2338860..98c3b9a 100644
--- a/src/gn/string_output_buffer.h
+++ b/src/gn/string_output_buffer.h
@@ -5,7 +5,8 @@
#ifndef TOOLS_GN_STRING_OUTPUT_BUFFER_H_
#define TOOLS_GN_STRING_OUTPUT_BUFFER_H_
-#include <array>
+#include <concepts>
+#include <cstring>
#include <memory>
#include <streambuf>
#include <string>
@@ -18,6 +19,27 @@
class Err;
+inline size_t MaximumSize(std::string_view s) {
+ return s.size();
+}
+inline void WriteToBuf(std::string_view s, char buf[], size_t& upto) {
+ memcpy(&buf[upto], s.data(), s.size());
+ upto += s.size();
+}
+
+inline size_t MaximumSize(char) {
+ return 1;
+}
+inline void WriteToBuf(char c, char buf[], size_t& upto) {
+ buf[upto++] = c;
+}
+
+template <typename T>
+concept Writable = requires(const T& val, char buf[], size_t& upto) {
+ { MaximumSize(val) } -> std::convertible_to<size_t>;
+ WriteToBuf(val, buf, upto);
+};
+
// An append-only very large storage area for string data. Useful for the parts
// of GN that need to generate huge output files (e.g. --ide=json will create
// a 139 MiB project.json file for the Fuchsia build).
@@ -48,15 +70,19 @@
std::string str() const;
// Return the number of characters stored in this instance.
- size_t size() const { return (pages_.size() - 1u) * kPageSize + pos_; }
+ size_t size() const { return ro_size_ + rw_.size; }
- // Append string to this instance.
- void Append(const char* str, size_t len);
- void Append(std::string_view str);
- void Append(char c);
+ // Append a value to this instance.
+ void Append(const Writable auto& value) {
+ Reserve(MaximumSize(value));
+ WriteToBuf(value, rw_.buf.get(), rw_.size);
+ }
+ void Append(const char* str, size_t len) {
+ Append(std::string_view(str, len));
+ }
- StringOutputBuffer& operator<<(std::string_view str) {
- Append(str);
+ StringOutputBuffer& operator<<(const Writable auto& value) {
+ Append(value);
return *this;
}
@@ -87,13 +113,29 @@
private:
// Return the number of free bytes in the current page.
- size_t page_free_size() const { return kPageSize - pos_; }
+ size_t page_free_size() const { return rw_capacity_ - rw_.size; }
+
+ // Ensures that the read-write section of the buffer has at least n bytes
+ // available.
+ void Reserve(size_t n);
static constexpr size_t kPageSize = 65536;
- using Page = std::array<char, kPageSize>;
- size_t pos_ = kPageSize;
- std::vector<std::unique_ptr<Page>> pages_;
+ struct Page {
+ std::unique_ptr<char[]> buf;
+ size_t size = 0;
+ };
+
+ // Because this is an extremely hot path, we avoid pointer indirection by
+ // storing the current page inline.
+ // The current page is read-write.
+ Page rw_;
+ size_t rw_capacity_ = 0;
+
+ // Completed read-only pages.
+ std::vector<Page> ro_;
+ // Contains sum([page.size for page in ro_])
+ size_t ro_size_ = 0;
};
#endif // TOOLS_GN_STRING_OUTPUT_BUFFER_H_
diff --git a/src/gn/string_output_buffer_unittest.cc b/src/gn/string_output_buffer_unittest.cc
index 6dcb741..9067fd1 100644
--- a/src/gn/string_output_buffer_unittest.cc
+++ b/src/gn/string_output_buffer_unittest.cc
@@ -132,3 +132,38 @@
ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
ASSERT_TRUE(buffer.ContentsEqual(file_path));
}
+
+namespace {
+
+struct CustomGreeting {
+ std::string_view name;
+};
+
+struct Unwritable {};
+
+size_t MaximumSize(const CustomGreeting& greeting) {
+ return greeting.name.size() + 7;
+}
+
+void WriteToBuf(const CustomGreeting& greeting, char* buf, size_t& size) {
+ constexpr std::string_view prefix = "Hello, ";
+ memcpy(buf + size, prefix.data(), prefix.size());
+ size += prefix.size();
+ memcpy(buf + size, greeting.name.data(), greeting.name.size());
+ size += greeting.name.size();
+}
+
+static_assert(Writable<CustomGreeting>);
+static_assert(Writable<std::string_view>);
+static_assert(Writable<char>);
+static_assert(Writable<std::string>);
+static_assert(Writable<const char*>);
+static_assert(!Writable<Unwritable>);
+
+} // namespace
+
+TEST(StringOutputBuffer, CustomWritable) {
+ StringOutputBuffer buffer;
+ buffer << CustomGreeting{"world"} << '!';
+ EXPECT_EQ(buffer.str(), "Hello, world!");
+}
