summaryrefslogtreecommitdiffstats
path: root/third_party
diff options
context:
space:
mode:
authordgrogan@chromium.org <dgrogan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-01-12 19:35:10 +0000
committerdgrogan@chromium.org <dgrogan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-01-12 19:35:10 +0000
commita99795043a290bbb4ec92068e3d9c68f6ebd15fd (patch)
treea84ba592f4db8cda4ceb04a28077400db3ea5185 /third_party
parent0d419a6f01abe2ea5e950335e89b80a4ea279456 (diff)
downloadchromium_src-a99795043a290bbb4ec92068e3d9c68f6ebd15fd.zip
chromium_src-a99795043a290bbb4ec92068e3d9c68f6ebd15fd.tar.gz
chromium_src-a99795043a290bbb4ec92068e3d9c68f6ebd15fd.tar.bz2
Fix LevelDB histogram code.
The old code misused the macros provided by histogram.h such that a DCHECK failed on the bots. UMA_HISTOGRAM_ENUMERATION et al use a static variable to retrieve the Histogram objects, so a single instance of that macro can't be used to retrieve multiple histograms. The old code tried to retrieve both LevelDBEnv.IOError and LevelDBEnv.IDB.IOError. This CL forgoes use of the macro and keeps a pointer to the Histogram objects around. http://wkb.ug/106135 will have to reland to make use of this. BUG=164755 Review URL: https://chromiumcodereview.appspot.com/11860015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176577 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'third_party')
-rw-r--r--third_party/leveldatabase/env_chromium.cc96
1 files changed, 51 insertions, 45 deletions
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc
index ac5fb53..9f25393 100644
--- a/third_party/leveldatabase/env_chromium.cc
+++ b/third_party/leveldatabase/env_chromium.cc
@@ -127,32 +127,11 @@ enum UmaEntry {
class UMALogger {
public:
- UMALogger(std::string uma_title);
- void RecordErrorAt(UmaEntry entry) const;
- void LogRandomAccessFileError(base::PlatformFileError error_code) const;
-
- private:
- std::string uma_title_;
+ virtual void RecordErrorAt(UmaEntry entry) const = 0;
+ virtual void LogRandomAccessFileError(base::PlatformFileError error_code)
+ const = 0;
};
-UMALogger::UMALogger(std::string uma_title) : uma_title_(uma_title) {}
-
-void UMALogger::RecordErrorAt(UmaEntry entry) const {
- std::string uma_name(uma_title_);
- uma_name.append(".IOError");
- UMA_HISTOGRAM_ENUMERATION(uma_name, entry, kNumEntries);
-}
-
-void UMALogger::LogRandomAccessFileError(base::PlatformFileError error_code)
- const {
- DCHECK(error_code < 0);
- std::string uma_name(uma_title_);
- uma_name.append(".IOError.RandomAccessFile");
- UMA_HISTOGRAM_ENUMERATION(uma_name,
- -error_code,
- -base::PLATFORM_FILE_ERROR_MAX);
-}
-
} // namespace
namespace leveldb {
@@ -339,7 +318,7 @@ class ChromiumFileLock : public FileLock {
::base::PlatformFile file_;
};
-class ChromiumEnv : public Env {
+class ChromiumEnv : public Env, public UMALogger {
public:
ChromiumEnv();
virtual ~ChromiumEnv() {
@@ -351,10 +330,10 @@ class ChromiumEnv : public Env {
FILE* f = fopen_internal(fname.c_str(), "rb");
if (f == NULL) {
*result = NULL;
- uma_logger_->RecordErrorAt(kNewSequentialFile);
+ RecordErrorAt(kNewSequentialFile);
return Status::IOError(fname, strerror(errno));
} else {
- *result = new ChromiumSequentialFile(fname, f, uma_logger_.get());
+ *result = new ChromiumSequentialFile(fname, f, this);
return Status::OK();
}
}
@@ -368,11 +347,11 @@ class ChromiumEnv : public Env {
CreateFilePath(fname), flags, &created, &error_code);
if (error_code != ::base::PLATFORM_FILE_OK) {
*result = NULL;
- uma_logger_->RecordErrorAt(kNewRandomAccessFile);
- uma_logger_->LogRandomAccessFileError(error_code);
+ RecordErrorAt(kNewRandomAccessFile);
+ LogRandomAccessFileError(error_code);
return Status::IOError(fname, PlatformFileErrorString(error_code));
}
- *result = new ChromiumRandomAccessFile(fname, file, uma_logger_.get());
+ *result = new ChromiumRandomAccessFile(fname, file, this);
return Status::OK();
}
@@ -381,14 +360,14 @@ class ChromiumEnv : public Env {
*result = NULL;
FILE* f = fopen_internal(fname.c_str(), "wb");
if (f == NULL) {
- uma_logger_->RecordErrorAt(kNewWritableFile);
+ RecordErrorAt(kNewWritableFile);
return Status::IOError(fname, strerror(errno));
} else {
if (!sync_parent(fname)) {
fclose(f);
return Status::IOError(fname, strerror(errno));
}
- *result = new ChromiumWritableFile(fname, f, uma_logger_.get());
+ *result = new ChromiumWritableFile(fname, f, this);
return Status::OK();
}
}
@@ -418,7 +397,7 @@ class ChromiumEnv : public Env {
// TODO(jorlow): Should we assert this is a file?
if (!::file_util::Delete(CreateFilePath(fname), false)) {
result = Status::IOError(fname, "Could not delete file.");
- uma_logger_->RecordErrorAt(kDeleteFile);
+ RecordErrorAt(kDeleteFile);
}
return result;
};
@@ -427,7 +406,7 @@ class ChromiumEnv : public Env {
Status result;
if (!::file_util::CreateDirectory(CreateFilePath(name))) {
result = Status::IOError(name, "Could not create directory.");
- uma_logger_->RecordErrorAt(kCreateDir);
+ RecordErrorAt(kCreateDir);
}
return result;
};
@@ -437,7 +416,7 @@ class ChromiumEnv : public Env {
// TODO(jorlow): Should we assert this is a directory?
if (!::file_util::Delete(CreateFilePath(name), false)) {
result = Status::IOError(name, "Could not delete directory.");
- uma_logger_->RecordErrorAt(kDeleteDir);
+ RecordErrorAt(kDeleteDir);
}
return result;
};
@@ -448,7 +427,7 @@ class ChromiumEnv : public Env {
if (!::file_util::GetFileSize(CreateFilePath(fname), &signed_size)) {
*size = 0;
s = Status::IOError(fname, "Could not determine file size.");
- uma_logger_->RecordErrorAt(kGetFileSize);
+ RecordErrorAt(kGetFileSize);
} else {
*size = static_cast<uint64_t>(signed_size);
}
@@ -459,7 +438,7 @@ class ChromiumEnv : public Env {
Status result;
if (!::file_util::ReplaceFile(CreateFilePath(src), CreateFilePath(dst))) {
result = Status::IOError(src, "Could not rename file.");
- uma_logger_->RecordErrorAt(kRenamefile);
+ RecordErrorAt(kRenamefile);
} else {
sync_parent(dst);
if (src != dst)
@@ -482,7 +461,7 @@ class ChromiumEnv : public Env {
CreateFilePath(fname), flags, &created, &error_code);
if (error_code != ::base::PLATFORM_FILE_OK) {
result = Status::IOError(fname, PlatformFileErrorString(error_code));
- uma_logger_->RecordErrorAt(kLockFile);
+ RecordErrorAt(kLockFile);
} else {
ChromiumFileLock* my_lock = new ChromiumFileLock;
my_lock->file_ = file;
@@ -496,7 +475,7 @@ class ChromiumEnv : public Env {
Status result;
if (!::base::ClosePlatformFile(my_lock->file_)) {
result = Status::IOError("Could not close lock file.");
- uma_logger_->RecordErrorAt(kUnlockFile);
+ RecordErrorAt(kUnlockFile);
}
delete my_lock;
return result;
@@ -525,7 +504,7 @@ class ChromiumEnv : public Env {
if (!::file_util::CreateNewTempDirectory(kLevelDBTestDirectoryPrefix,
&test_directory_)) {
mu_.Release();
- uma_logger_->RecordErrorAt(kGetTestDirectory);
+ RecordErrorAt(kGetTestDirectory);
return Status::IOError("Could not create temp directory.");
}
}
@@ -538,7 +517,7 @@ class ChromiumEnv : public Env {
FILE* f = fopen_internal(fname.c_str(), "w");
if (f == NULL) {
*result = NULL;
- uma_logger_->RecordErrorAt(kNewLogger);
+ RecordErrorAt(kNewLogger);
return Status::IOError(fname, strerror(errno));
} else {
if (!sync_parent(fname)) {
@@ -559,8 +538,17 @@ class ChromiumEnv : public Env {
::base::PlatformThread::Sleep(::base::TimeDelta::FromMicroseconds(micros));
}
+ void RecordErrorAt(UmaEntry entry) const {
+ io_error_histogram_->Add(entry);
+ }
+
+ void LogRandomAccessFileError(base::PlatformFileError error_code) const {
+ DCHECK(error_code < 0);
+ random_access_file_histogram_->Add(-error_code);
+ }
+
protected:
- scoped_ptr<UMALogger> uma_logger_;
+ void InitHistograms(const std::string& uma_title);
private:
// BGThread() is the body of the background thread
@@ -580,13 +568,31 @@ class ChromiumEnv : public Env {
struct BGItem { void* arg; void (*function)(void*); };
typedef std::deque<BGItem> BGQueue;
BGQueue queue_;
+
+ base::Histogram* io_error_histogram_;
+ base::Histogram* random_access_file_histogram_;
};
ChromiumEnv::ChromiumEnv()
: page_size_(::base::SysInfo::VMAllocationGranularity()),
bgsignal_(&mu_),
- started_bgthread_(false),
- uma_logger_(new UMALogger("LevelDBEnv")) {
+ started_bgthread_(false) {
+ InitHistograms("LevelDBEnv");
+}
+
+void ChromiumEnv::InitHistograms(const std::string& uma_title) {
+ std::string uma_name(uma_title);
+ uma_name.append(".IOError");
+ // Note: The calls to FactoryGet aren't thread-safe. It's ok to call them here
+ // because this method is only called from LazyInstance, which provides
+ // thread-safety.
+ io_error_histogram_ = base::LinearHistogram::FactoryGet(uma_name, 1,
+ kNumEntries, kNumEntries + 1, base::Histogram::kUmaTargetedHistogramFlag);
+
+ uma_name.append(".RandomAccessFile");
+ random_access_file_histogram_ = base::LinearHistogram::FactoryGet(uma_name, 1,
+ -base::PLATFORM_FILE_ERROR_MAX, -base::PLATFORM_FILE_ERROR_MAX + 1,
+ base::Histogram::kUmaTargetedHistogramFlag);
}
class Thread : public ::base::PlatformThread::Delegate {
@@ -655,7 +661,7 @@ void ChromiumEnv::StartThread(void (*function)(void* arg), void* arg) {
class IDBEnv : public ChromiumEnv {
public:
IDBEnv() : ChromiumEnv() {
- uma_logger_.reset(new UMALogger("LevelDBEnv.IDB"));
+ InitHistograms("LevelDBEnv.IDB");
}
};