Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
Read in file-cache entries all at once, rather than a chunk at a time.
Browse files Browse the repository at this point in the history
When reading large images a chunk at a time, callgrind indicated a
significant amount of time spent appending bytes to a dynamically
expanding buffer.

Note: this adds a call to fstat so we know the size of the file before
we start to read it.

Another note: the only way I could find to implement this reading into
std::string involved using std::string::resize, which nulls the bytes we
are about to write.  That seems like a shame but it is what it is :).
  • Loading branch information
jmarantz committed Nov 25, 2015
1 parent 1ad70a5 commit 56e4c8d
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 4 deletions.
1 change: 1 addition & 0 deletions net/instaweb/test.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@
'rewriter/javascript_minify_speed_test.cc',
'rewriter/rewrite_driver_speed_test.cc',
'<(DEPTH)/pagespeed/kernel/base/fast_wildcard_group_speed_test.cc',
'<(DEPTH)/pagespeed/kernel/base/file_system_speed_test.cc',
'<(DEPTH)/pagespeed/kernel/base/string_multi_map_speed_test.cc',
'<(DEPTH)/pagespeed/kernel/base/wildcard_group.cc',
'<(DEPTH)/pagespeed/kernel/cache/compressed_cache_speed_test.cc',
Expand Down
12 changes: 8 additions & 4 deletions pagespeed/kernel/base/file_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ int FileSystem::MaxPathLength(const StringPiece& base) const {

bool FileSystem::ReadFile(const char* filename, GoogleString* buffer,
MessageHandler* message_handler) {
StringWriter writer(buffer);
return ReadFile(filename, &writer, message_handler);
InputFile* input_file = OpenInputFile(filename, message_handler);
return ReadFile(input_file, buffer, message_handler);
}

bool FileSystem::ReadFile(const char* filename, Writer* writer,
Expand All @@ -61,8 +61,12 @@ bool FileSystem::ReadFile(const char* filename, Writer* writer,

bool FileSystem::ReadFile(InputFile* input_file, GoogleString* buffer,
MessageHandler* message_handler) {
StringWriter writer(buffer);
return ReadFile(input_file, &writer, message_handler);
bool ret = false;
if (input_file != NULL) {
ret = input_file->ReadFile(buffer, message_handler);
ret &= Close(input_file, message_handler);
}
return ret;
}

bool FileSystem::ReadFile(InputFile* input_file, Writer* writer,
Expand Down
3 changes: 3 additions & 0 deletions pagespeed/kernel/base/file_system.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class FileSystem {
// Note: This returns num bytes read, NOT a success bool.
virtual int Read(char* buf, int size, MessageHandler* handler) = 0;

// Reads entire file into buf, returning true if successful.
virtual bool ReadFile(GoogleString* buf, MessageHandler* handler) = 0;

protected:
friend class FileSystem;
virtual ~InputFile();
Expand Down
107 changes: 107 additions & 0 deletions pagespeed/kernel/base/file_system_speed_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Copyright 2015 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// Author: jmarantz@google.com (Joshua Marantz)

#include "base/logging.h"
#include "pagespeed/kernel/base/basictypes.h"
#include "pagespeed/kernel/base/benchmark.h"
#include "pagespeed/kernel/base/google_message_handler.h"
#include "pagespeed/kernel/base/gtest.h"
#include "pagespeed/kernel/base/stdio_file_system.h"
#include "pagespeed/kernel/base/string.h"
#include "pagespeed/kernel/base/string_util.h"
#include "pagespeed/kernel/base/string_writer.h"

// Running the speed test:
// src/out/Release/mod_pagespeed_speed_test .File
// BM_100kWholeFile 100000 18845 ns/op
// BM_100kStreamingFile 50000 70181 ns/op
// BM_1MWholeFile 10000 122070 ns/op
// BM_1MStreamingFile 2000 760416 ns/op
//

namespace net_instaweb {

namespace {

class FSTester {
public:
explicit FSTester(int size) {
StopBenchmarkTiming();
GoogleString str;
str.append(size, 'a');
filename_ = StrCat(GTestTempDir(), "large_file.txt");
file_system_.WriteFile(filename_.c_str(), str, &handler_);
}

~FSTester() {
file_system_.RemoveFile(filename_.c_str(), &handler_);
StartBenchmarkTiming();
}

void ReadWholeFile(int iters) {
StartBenchmarkTiming();
for (int i = 0; i < iters; ++i) {
GoogleString buf;
CHECK(file_system_.ReadFile(filename_.c_str(), &buf, &handler_));
}
StopBenchmarkTiming();
}

void StreamingReadFile(int iters) {
StartBenchmarkTiming();
for (int i = 0; i < iters; ++i) {
GoogleString buf;
StringWriter writer(&buf);
CHECK(file_system_.ReadFile(filename_.c_str(), &writer, &handler_));
}
StopBenchmarkTiming();
}

private:
StdioFileSystem file_system_;
GoogleString filename_;
GoogleMessageHandler handler_;
};

static void BM_100kWholeFile(int iters) {
FSTester fs_tester(100000);
fs_tester.ReadWholeFile(iters);
}
BENCHMARK(BM_100kWholeFile);

static void BM_100kStreamingFile(int iters) {
FSTester fs_tester(100000);
fs_tester.StreamingReadFile(iters);
}
BENCHMARK(BM_100kStreamingFile);

static void BM_1MWholeFile(int iters) {
FSTester fs_tester(1000000);
fs_tester.ReadWholeFile(iters);
}
BENCHMARK(BM_1MWholeFile);

static void BM_1MStreamingFile(int iters) {
FSTester fs_tester(1000000);
fs_tester.StreamingReadFile(iters);
}
BENCHMARK(BM_1MStreamingFile);

} // namespace

} // namespace net_instaweb
5 changes: 5 additions & 0 deletions pagespeed/kernel/base/mem_file_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class MemInputFile : public FileSystem::InputFile {
return size;
}

virtual bool ReadFile(GoogleString* buf, MessageHandler* message_handler) {
*buf = contents_;
return true;
}

private:
const GoogleString contents_;
const GoogleString filename_;
Expand Down
19 changes: 19 additions & 0 deletions pagespeed/kernel/base/stdio_file_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,25 @@ class StdioInputFile : public FileSystem::InputFile {
: file_helper_(f, filename, fs) {
}

virtual bool ReadFile(GoogleString* buf, MessageHandler* message_handler) {
bool ret = false;
struct stat statbuf;
file_helper_.StartTimer();
if ((fstat(fileno(file_helper_.file_), &statbuf) < 0)) {
file_helper_.ReportError(message_handler, "stating file: %s");
} else {
buf->resize(statbuf.st_size);
int nread = fread(&(*buf)[0], 1, statbuf.st_size, file_helper_.file_);
if (nread != statbuf.st_size) {
file_helper_.ReportError(message_handler, "reading file: %s");
} else {
ret = true;
}
}
file_helper_.EndTimer("ReadFile");
return ret;
}

virtual int Read(char* buf, int size, MessageHandler* message_handler) {
file_helper_.StartTimer();
int ret = fread(buf, 1, size, file_helper_.file_);
Expand Down

0 comments on commit 56e4c8d

Please sign in to comment.