Use StringOutputBuffer to write file
The class StringOutputBuffer performs the comparison between the
memory buffer and the destination file in chunks instead of reading
the file in one huge string. This is slightly more performant when
generating Chromium build files for iOS and macOS (~ 4% faster).
Bug: none
Change-Id: Iba763c5f58bf16233cc3e6833a3b38294dce2de8
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/11401
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/compile_commands_writer.cc b/src/gn/compile_commands_writer.cc
index 9f8ef63..1d9d5b4 100644
--- a/src/gn/compile_commands_writer.cc
+++ b/src/gn/compile_commands_writer.cc
@@ -15,7 +15,6 @@
#include "gn/config_values_extractors.h"
#include "gn/deps_iterator.h"
#include "gn/escape.h"
-#include "gn/filesystem_utils.h"
#include "gn/ninja_target_command_util.h"
#include "gn/path_output.h"
#include "gn/string_output_buffer.h"
@@ -324,11 +323,7 @@
OutputJSON(build_settings, preserved_targets, output_to_json);
}
- if (!json.ContentsEqual(output_path)) {
- if (!json.WriteToFile(output_path, err))
- return false;
- }
- return true;
+ return json.WriteToFileIfChanged(output_path, err);
}
std::vector<const Target*> CompileCommandsWriter::FilterTargets(
diff --git a/src/gn/filesystem_utils.cc b/src/gn/filesystem_utils.cc
index 4906d56..6471378 100644
--- a/src/gn/filesystem_utils.cc
+++ b/src/gn/filesystem_utils.cc
@@ -939,15 +939,6 @@
return file_data == data;
}
-bool WriteFileIfChanged(const base::FilePath& file_path,
- const std::string& data,
- Err* err) {
- if (ContentsEqual(file_path, data))
- return true;
-
- return WriteFile(file_path, data, err);
-}
-
bool WriteFile(const base::FilePath& file_path,
const std::string& data,
Err* err) {
diff --git a/src/gn/filesystem_utils.h b/src/gn/filesystem_utils.h
index 9f21cff..2c826b5 100644
--- a/src/gn/filesystem_utils.h
+++ b/src/gn/filesystem_utils.h
@@ -203,14 +203,6 @@
// otherwise.
bool ContentsEqual(const base::FilePath& file_path, const std::string& data);
-// Writes given stream contents to the given file if it differs from existing
-// file contents. Returns true if new contents was successfully written or
-// existing file contents doesn't need updating, false on write error. |err| is
-// set on write error if not nullptr.
-bool WriteFileIfChanged(const base::FilePath& file_path,
- const std::string& data,
- Err* err);
-
// Writes given stream contents to the given file. Returns true if data was
// successfully written, false otherwise. |err| is set on error if not nullptr.
bool WriteFile(const base::FilePath& file_path,
diff --git a/src/gn/filesystem_utils_unittest.cc b/src/gn/filesystem_utils_unittest.cc
index 347d0fe..0eb786d 100644
--- a/src/gn/filesystem_utils_unittest.cc
+++ b/src/gn/filesystem_utils_unittest.cc
@@ -638,43 +638,6 @@
EXPECT_FALSE(ContentsEqual(file_path, "bar"));
}
-TEST(FilesystemUtils, WriteFileIfChanged) {
- base::ScopedTempDir temp_dir;
- ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
- std::string data = "foo";
-
- // Write if file doesn't exist. Create also directory.
- base::FilePath file_path =
- temp_dir.GetPath().AppendASCII("bar").AppendASCII("foo.txt");
- EXPECT_TRUE(WriteFileIfChanged(file_path, data, nullptr));
-
- base::File::Info file_info;
- ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
- Ticks last_modified = file_info.last_modified;
-
- {
- using namespace std::chrono_literals;
-#if defined(OS_MACOSX)
- // Modification times are in seconds in HFS on Mac.
- std::this_thread::sleep_for(1s);
-#else
- std::this_thread::sleep_for(1ms);
-#endif
- }
-
- // Don't write if contents is the same.
- EXPECT_TRUE(WriteFileIfChanged(file_path, data, nullptr));
- ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
- EXPECT_EQ(last_modified, file_info.last_modified);
-
- // Write if contents changed.
- EXPECT_TRUE(WriteFileIfChanged(file_path, "bar", nullptr));
- std::string file_data;
- ASSERT_TRUE(base::ReadFileToString(file_path, &file_data));
- EXPECT_EQ("bar", file_data);
-}
-
TEST(FilesystemUtils, GetToolchainDirs) {
BuildSettings build_settings;
build_settings.SetBuildDir(SourceDir("//out/Debug/"));
diff --git a/src/gn/function_write_file.cc b/src/gn/function_write_file.cc
index 2c568c0..b0884da 100644
--- a/src/gn/function_write_file.cc
+++ b/src/gn/function_write_file.cc
@@ -8,6 +8,7 @@
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
+#include "gn/build_settings.h"
#include "gn/err.h"
#include "gn/filesystem_utils.h"
#include "gn/functions.h"
@@ -15,6 +16,7 @@
#include "gn/output_conversion.h"
#include "gn/parse_tree.h"
#include "gn/scheduler.h"
+#include "gn/string_output_buffer.h"
#include "util/build_config.h"
namespace functions {
@@ -87,7 +89,8 @@
output_conversion = args[2];
// Compute output.
- std::ostringstream contents;
+ StringOutputBuffer storage;
+ std::ostream contents(&storage);
ConvertValueToOutput(scope->settings(), args[1], output_conversion, contents,
err);
if (err->has_error())
@@ -97,8 +100,10 @@
scope->settings()->build_settings()->GetFullPath(source_file);
// Make sure we're not replacing the same contents.
- if (!WriteFileIfChanged(file_path, contents.str(), err))
+ if (!storage.WriteToFileIfChanged(file_path, err)) {
*err = Err(function->function(), err->message(), err->help_text());
+ return Value();
+ }
return Value();
}
diff --git a/src/gn/ninja_generated_file_target_writer.cc b/src/gn/ninja_generated_file_target_writer.cc
index afe1e08..e1ad19f 100644
--- a/src/gn/ninja_generated_file_target_writer.cc
+++ b/src/gn/ninja_generated_file_target_writer.cc
@@ -6,11 +6,11 @@
#include "base/strings/string_util.h"
#include "gn/deps_iterator.h"
-#include "gn/filesystem_utils.h"
#include "gn/output_conversion.h"
#include "gn/output_file.h"
#include "gn/scheduler.h"
#include "gn/settings.h"
+#include "gn/string_output_buffer.h"
#include "gn/string_utils.h"
#include "gn/target.h"
#include "gn/trace.h"
@@ -79,7 +79,8 @@
ScopedTrace trace(TraceItem::TRACE_FILE_WRITE, outputs_as_sources[0].value());
// Compute output.
- std::ostringstream out;
+ StringOutputBuffer storage;
+ std::ostream out(&storage);
ConvertValueToOutput(settings_, contents, target_->output_conversion(), out,
&err);
@@ -88,7 +89,7 @@
return;
}
- WriteFileIfChanged(output, out.str(), &err);
+ storage.WriteToFileIfChanged(output, &err);
if (err.has_error()) {
g_scheduler->FailWithError(err);
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc
index c8e7ebd..e85400f 100644
--- a/src/gn/ninja_target_writer.cc
+++ b/src/gn/ninja_target_writer.cc
@@ -23,6 +23,7 @@
#include "gn/ninja_utils.h"
#include "gn/output_file.h"
#include "gn/scheduler.h"
+#include "gn/string_output_buffer.h"
#include "gn/string_utils.h"
#include "gn/substitution_writer.h"
#include "gn/target.h"
@@ -51,7 +52,8 @@
// It's ridiculously faster to write to a string and then write that to
// disk in one operation than to use an fstream here.
- std::stringstream rules;
+ StringOutputBuffer storage;
+ std::ostream rules(&storage);
// Call out to the correct sub-type of writer. Binary targets need to be
// written to separate files for compiler flag scoping, but other target
@@ -101,8 +103,7 @@
SourceFile ninja_file = GetNinjaFileForTarget(target);
base::FilePath full_ninja_file =
settings->build_settings()->GetFullPath(ninja_file);
- base::CreateDirectory(full_ninja_file.DirName());
- WriteFileIfChanged(full_ninja_file, rules.str(), nullptr);
+ storage.WriteToFileIfChanged(full_ninja_file, nullptr);
EscapeOptions options;
options.mode = ESCAPE_NINJA;
@@ -117,7 +118,7 @@
}
// No separate file required, just return the rules.
- return rules.str();
+ return storage.str();
}
void NinjaTargetWriter::WriteEscapedSubstitution(const Substitution* type) {
diff --git a/src/gn/qt_creator_writer.cc b/src/gn/qt_creator_writer.cc
index f5c997a..b0fa05c 100644
--- a/src/gn/qt_creator_writer.cc
+++ b/src/gn/qt_creator_writer.cc
@@ -19,6 +19,7 @@
#include "gn/filesystem_utils.h"
#include "gn/label.h"
#include "gn/loader.h"
+#include "gn/string_output_buffer.h"
namespace {
base::FilePath::CharType kProjectDirName[] =
@@ -270,10 +271,11 @@
void QtCreatorWriter::GenerateFile(const base::FilePath::CharType* suffix,
const std::set<std::string>& items) {
const base::FilePath file_path = project_prefix_.AddExtension(suffix);
- std::ostringstream output;
+ StringOutputBuffer storage;
+ std::ostream output(&storage);
for (const std::string& item : items)
output << item << std::endl;
- WriteFileIfChanged(file_path, output.str(), &err_);
+ storage.WriteToFileIfChanged(file_path, &err_);
}
void QtCreatorWriter::Run() {
diff --git a/src/gn/runtime_deps.cc b/src/gn/runtime_deps.cc
index 3b6d683..25eb2ef 100644
--- a/src/gn/runtime_deps.cc
+++ b/src/gn/runtime_deps.cc
@@ -19,6 +19,7 @@
#include "gn/output_file.h"
#include "gn/scheduler.h"
#include "gn/settings.h"
+#include "gn/string_output_buffer.h"
#include "gn/switches.h"
#include "gn/target.h"
#include "gn/trace.h"
@@ -202,12 +203,13 @@
base::FilePath data_deps_file =
target->settings()->build_settings()->GetFullPath(output_as_source);
- std::stringstream contents;
+ StringOutputBuffer storage;
+ std::ostream contents(&storage);
for (const auto& pair : ComputeRuntimeDeps(target))
contents << pair.first.value() << std::endl;
ScopedTrace trace(TraceItem::TRACE_FILE_WRITE, output_as_source.value());
- return WriteFileIfChanged(data_deps_file, contents.str(), err);
+ return storage.WriteToFileIfChanged(data_deps_file, err);
}
} // namespace
diff --git a/src/gn/rust_project_writer.cc b/src/gn/rust_project_writer.cc
index ee62479..c11f967 100644
--- a/src/gn/rust_project_writer.cc
+++ b/src/gn/rust_project_writer.cc
@@ -12,7 +12,6 @@
#include "base/json/string_escape.h"
#include "gn/builder.h"
#include "gn/deps_iterator.h"
-#include "gn/filesystem_utils.h"
#include "gn/ninja_target_command_util.h"
#include "gn/rust_project_writer_helpers.h"
#include "gn/rust_tool.h"
@@ -70,12 +69,7 @@
std::ostream out(&out_buffer);
RenderJSON(build_settings, all_targets, out);
-
- if (out_buffer.ContentsEqual(output_path)) {
- return true;
- }
-
- return out_buffer.WriteToFile(output_path, err);
+ return out_buffer.WriteToFileIfChanged(output_path, err);
}
// Map of Targets to their index in the crates list (for linking dependencies to
diff --git a/src/gn/string_output_buffer.cc b/src/gn/string_output_buffer.cc
index 09913ce..c5d91ff 100644
--- a/src/gn/string_output_buffer.cc
+++ b/src/gn/string_output_buffer.cc
@@ -115,3 +115,11 @@
}
return success;
}
+
+bool StringOutputBuffer::WriteToFileIfChanged(const base::FilePath& file_path,
+ Err* err) const {
+ if (ContentsEqual(file_path))
+ return true;
+
+ return WriteToFile(file_path, err);
+}
diff --git a/src/gn/string_output_buffer.h b/src/gn/string_output_buffer.h
index 5e89250..2338860 100644
--- a/src/gn/string_output_buffer.h
+++ b/src/gn/string_output_buffer.h
@@ -66,6 +66,10 @@
// Write the contents of this instance to a file at |file_path|.
bool WriteToFile(const base::FilePath& file_path, Err* err) const;
+ // Write the contents of this instance to a file at |file_path| unless the
+ // file already exists and the contents are equal.
+ bool WriteToFileIfChanged(const base::FilePath& file_path, Err* err) const;
+
static size_t GetPageSizeForTesting() { return kPageSize; }
protected:
diff --git a/src/gn/visual_studio_writer.cc b/src/gn/visual_studio_writer.cc
index 6ee56e8..f6ea9d2 100644
--- a/src/gn/visual_studio_writer.cc
+++ b/src/gn/visual_studio_writer.cc
@@ -25,6 +25,7 @@
#include "gn/parse_tree.h"
#include "gn/path_output.h"
#include "gn/standard_out.h"
+#include "gn/string_output_buffer.h"
#include "gn/target.h"
#include "gn/variables.h"
#include "gn/visual_studio_utils.h"
@@ -411,7 +412,8 @@
FilePathToUTF8(build_settings_->GetFullPath(target->label().dir())),
project_config_platform));
- std::stringstream vcxproj_string_out;
+ StringOutputBuffer vcxproj_storage;
+ std::ostream vcxproj_string_out(&vcxproj_storage);
SourceFileCompileTypePairs source_types;
if (!WriteProjectFileContents(vcxproj_string_out, *projects_.back(), target,
ninja_extra_args, &source_types, err)) {
@@ -422,13 +424,15 @@
// Only write the content to the file if it's different. That is
// both a performance optimization and more importantly, prevents
// Visual Studio from reloading the projects.
- if (!WriteFileIfChanged(vcxproj_path, vcxproj_string_out.str(), err))
+ if (!vcxproj_storage.WriteToFileIfChanged(vcxproj_path, err))
return false;
base::FilePath filters_path = UTF8ToFilePath(vcxproj_path_str + ".filters");
- std::stringstream filters_string_out;
+
+ StringOutputBuffer filters_storage;
+ std::ostream filters_string_out(&filters_storage);
WriteFiltersFileContents(filters_string_out, target, source_types);
- return WriteFileIfChanged(filters_path, filters_string_out.str(), err);
+ return filters_storage.WriteToFileIfChanged(filters_path, err);
}
bool VisualStudioWriter::WriteProjectFileContents(
@@ -732,13 +736,14 @@
base::FilePath sln_path = build_settings_->GetFullPath(sln_file);
- std::stringstream string_out;
+ StringOutputBuffer storage;
+ std::ostream string_out(&storage);
WriteSolutionFileContents(string_out, sln_path.DirName());
// Only write the content to the file if it's different. That is
// both a performance optimization and more importantly, prevents
// Visual Studio from reloading the projects.
- return WriteFileIfChanged(sln_path, string_out.str(), err);
+ return storage.WriteToFileIfChanged(sln_path, err);
}
void VisualStudioWriter::WriteSolutionFileContents(
diff --git a/src/gn/xcode_writer.cc b/src/gn/xcode_writer.cc
index a5650fa..27f28e1 100644
--- a/src/gn/xcode_writer.cc
+++ b/src/gn/xcode_writer.cc
@@ -31,6 +31,7 @@
#include "gn/scheduler.h"
#include "gn/settings.h"
#include "gn/source_file.h"
+#include "gn/string_output_buffer.h"
#include "gn/substitution_writer.h"
#include "gn/target.h"
#include "gn/value.h"
@@ -489,7 +490,8 @@
if (source_file.is_null())
return false;
- std::stringstream out;
+ StringOutputBuffer storage;
+ std::ostream out(&storage);
out << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
<< "<Workspace\n"
<< " version = \"1.0\">\n"
@@ -498,8 +500,8 @@
<< " </FileRef>\n"
<< "</Workspace>\n";
- return WriteFileIfChanged(build_settings_->GetFullPath(source_file),
- out.str(), err);
+ return storage.WriteToFileIfChanged(build_settings_->GetFullPath(source_file),
+ err);
}
bool XcodeWorkspace::WriteSettingsFile(const std::string& name,
@@ -511,7 +513,8 @@
if (source_file.is_null())
return false;
- std::stringstream out;
+ StringOutputBuffer storage;
+ std::ostream out(&storage);
out << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
<< "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" "
<< "\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
@@ -530,8 +533,8 @@
out << "</dict>\n"
<< "</plist>\n";
- return WriteFileIfChanged(build_settings_->GetFullPath(source_file),
- out.str(), err);
+ return storage.WriteToFileIfChanged(build_settings_->GetFullPath(source_file),
+ err);
}
// Class responsible for constructing and writing the .xcodeproj from the
@@ -844,11 +847,12 @@
if (pbxproj_file.is_null())
return false;
- std::stringstream pbxproj_string_out;
+ StringOutputBuffer storage;
+ std::ostream pbxproj_string_out(&storage);
WriteFileContent(pbxproj_string_out);
- if (!WriteFileIfChanged(build_settings_->GetFullPath(pbxproj_file),
- pbxproj_string_out.str(), err)) {
+ if (!storage.WriteToFileIfChanged(build_settings_->GetFullPath(pbxproj_file),
+ err)) {
return false;
}
