diff options
author | dgrogan@chromium.org <dgrogan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-12 19:35:10 +0000 |
---|---|---|
committer | dgrogan@chromium.org <dgrogan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-12 19:35:10 +0000 |
commit | a99795043a290bbb4ec92068e3d9c68f6ebd15fd (patch) | |
tree | a84ba592f4db8cda4ceb04a28077400db3ea5185 /third_party | |
parent | 0d419a6f01abe2ea5e950335e89b80a4ea279456 (diff) | |
download | chromium_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.cc | 96 |
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"); } }; |