From 5afb412ddcacc1a835e32fd98c75affb0af72800 Mon Sep 17 00:00:00 2001 From: Noriyuki Takahashi Date: Sat, 30 May 2015 15:19:14 -0700 Subject: [PATCH] Resolve mutual dependency of base/mmap. Move SystemUtil::MaybeMLock and SystemUtil::MaybeMUnlock to Mmap to resolve the mutual dependency of base/mmap on base/file_util and base/system_util. By this CL, base/mmap becomes a single build target. BUG=none TEST=unittest --- src/base/file_util_test.cc | 1 - src/base/mmap.cc | 26 ++++++++++++++-- src/base/mmap.h | 22 ++++++++++++-- src/base/mmap_test.cc | 32 ++++++++++++++++++++ src/base/system_util.cc | 24 ++------------- src/base/system_util.h | 16 ---------- src/base/system_util_test.cc | 35 ---------------------- src/dictionary/system/system_dictionary.cc | 4 +-- src/dictionary/system/value_dictionary.cc | 4 +-- src/mozc_version_template.txt | 2 +- 10 files changed, 82 insertions(+), 84 deletions(-) diff --git a/src/base/file_util_test.cc b/src/base/file_util_test.cc index f33a31316..a9e5dacbb 100644 --- a/src/base/file_util_test.cc +++ b/src/base/file_util_test.cc @@ -37,7 +37,6 @@ #include "base/file_stream.h" #include "base/logging.h" -#include "base/mmap.h" #include "base/number_util.h" #include "base/util.h" #include "testing/base/public/gunit.h" diff --git a/src/base/mmap.cc b/src/base/mmap.cc index 72f94567d..90a343c84 100644 --- a/src/base/mmap.cc +++ b/src/base/mmap.cc @@ -44,7 +44,6 @@ #include "base/port.h" #include "base/logging.h" -#include "base/system_util.h" #include "base/util.h" #ifdef OS_WIN @@ -181,7 +180,7 @@ bool Mmap::Open(const char *filename, const char *mode) { return false; } - SystemUtil::MaybeMLock(ptr, size_); + MaybeMLock(ptr, size_); text_ = reinterpret_cast(ptr); size_ = st.st_size; return true; @@ -189,7 +188,7 @@ bool Mmap::Open(const char *filename, const char *mode) { void Mmap::Close() { if (text_ != NULL) { - SystemUtil::MaybeMUnlock(text_, size_); + MaybeMUnlock(text_, size_); munmap(reinterpret_cast(text_), size_); } @@ -256,4 +255,25 @@ bool Mmap::SyncToFile() { } #endif // MOZC_USE_PEPPER_FILE_IO +int Mmap::MaybeMLock(const void *addr, size_t len) { + // TODO(yukawa): Integrate mozc_cache service. +#if defined(OS_WIN) || defined(OS_ANDROID) || defined(__native_client__) + return -1; +#else // defined(OS_WIN) || defined(OS_ANDROID) || + // defined(__native_client__) + return mlock(addr, len); +#endif // defined(OS_WIN) || defined(OS_ANDROID) || + // defined(__native_client__) +} + +int Mmap::MaybeMUnlock(const void *addr, size_t len) { +#if defined(OS_WIN) || defined(OS_ANDROID) || defined(__native_client__) + return -1; +#else // defined(OS_WIN) || defined(OS_ANDROID) || + // defined(__native_client__) + return munlock(addr, len); +#endif // defined(OS_WIN) || defined(OS_ANDROID) || + // defined(__native_client__) +} + } // namespace mozc diff --git a/src/base/mmap.h b/src/base/mmap.h index 4ddc65816..966c6cc6b 100644 --- a/src/base/mmap.h +++ b/src/base/mmap.h @@ -38,15 +38,30 @@ #include "base/port.h" namespace mozc { + class Mmap { public: Mmap(); - ~Mmap() { - Close(); - } + ~Mmap() { Close(); } + bool Open(const char *filename, const char *mode = "r"); void Close(); + // Following mlock/munlock related functions work based on target environment. + // In the case of Android, Native Client, Windows, we don't want to call + // actual functions, so these functions do nothing and return -1. In other + // cases, these functions call actual mlock/munlock functions and return it's + // result. + // On Android, page-out is probably acceptable because + // - Smaller RAM on the device. + // - The storage is (usually) solid state thus page-in/out is expected to + // be faster. + // On Linux, in the kernel version >= 2.6.9, user process can mlock. In older + // kernel, it fails if the process is running in user priviledge. + // TODO(team): Check in mac that mlock is really necessary. + static int MaybeMLock(const void *addr, size_t len); + static int MaybeMUnlock(const void *addr, size_t len); + #ifndef MOZC_USE_PEPPER_FILE_IO char &operator[](size_t n) { return *(text_ + n); } char operator[](size_t n) const { return *(text_ + n); } @@ -82,6 +97,7 @@ class Mmap { DISALLOW_COPY_AND_ASSIGN(Mmap); }; + } // namespace mozc #endif // MOZC_BASE_MMAP_H_ diff --git a/src/base/mmap_test.cc b/src/base/mmap_test.cc index e1c1ec45c..0eed0866e 100644 --- a/src/base/mmap_test.cc +++ b/src/base/mmap_test.cc @@ -97,5 +97,37 @@ TEST(MmapTest, MmapTest) { FileUtil::Unlink(filename); } } + +#if defined(OS_WIN) +TEST(MmapTest, WindowsMaybeMLockTest) { + size_t data_len = 32; + void *addr = malloc(data_len); + EXPECT_EQ(-1, Mmap::MaybeMLock(addr, data_len)); + EXPECT_EQ(-1, Mmap::MaybeMUnlock(addr, data_len)); + free(addr); +} +#elif defined(OS_MACOSX) +TEST(MmapTest, MacMaybeMLockTest) { + size_t data_len = 32; + void *addr = malloc(data_len); + EXPECT_EQ(0, Mmap::MaybeMLock(addr, data_len)); + EXPECT_EQ(0, Mmap::MaybeMUnlock(addr, data_len)); + free(addr); +} +#else +TEST(MmapTest, LinuxMaybeMLockTest) { + size_t data_len = 32; + void *addr = malloc(data_len); +#if defined(OS_ANDROID) || defined(__native_client__) + EXPECT_EQ(-1, Mmap::MaybeMLock(addr, data_len)); + EXPECT_EQ(-1, Mmap::MaybeMUnlock(addr, data_len)); +#else + EXPECT_EQ(0, Mmap::MaybeMLock(addr, data_len)); + EXPECT_EQ(0, Mmap::MaybeMUnlock(addr, data_len)); +#endif // defined(OS_ANDROID) || defined(__native_client__) + free(addr); +} +#endif // OS_WIN, OS_MACOSX, else. + } // namespace } // namespace mozc diff --git a/src/base/system_util.cc b/src/base/system_util.cc index 02f9f9034..a26c9a4f1 100644 --- a/src/base/system_util.cc +++ b/src/base/system_util.cc @@ -66,6 +66,9 @@ #include "base/win_util.h" #ifdef OS_ANDROID +// HACK to avoid a bug in sysconf in android. +#include "android/jni/sysconf.h" +#define sysconf mysysconf #include "base/android_util.h" #endif // OS_ANDROID @@ -1072,25 +1075,4 @@ bool SystemUtil::IsLittleEndian() { #endif // OS_WIN } -int SystemUtil::MaybeMLock(const void *addr, size_t len) { - // TODO(yukawa): Integrate mozc_cache service. -#if defined(OS_WIN) || defined(OS_ANDROID) || defined(__native_client__) - return -1; -#else // defined(OS_WIN) || defined(OS_ANDROID) || - // defined(__native_client__) - return mlock(addr, len); -#endif // defined(OS_WIN) || defined(OS_ANDROID) || - // defined(__native_client__) -} - -int SystemUtil::MaybeMUnlock(const void *addr, size_t len) { -#if defined(OS_WIN) || defined(OS_ANDROID) || defined(__native_client__) - return -1; -#else // defined(OS_WIN) || defined(OS_ANDROID) || - // defined(__native_client__) - return munlock(addr, len); -#endif // defined(OS_WIN) || defined(OS_ANDROID) || - // defined(__native_client__) -} - } // namespace mozc diff --git a/src/base/system_util.h b/src/base/system_util.h index 25c54d5c0..7b0d97cf5 100644 --- a/src/base/system_util.h +++ b/src/base/system_util.h @@ -209,22 +209,6 @@ class SystemUtil { // check endian-ness at runtime. static bool IsLittleEndian(); - // Following mlock/munlock related functions work based on target environment. - // In the case of Android, Native Client, Windows, we don't want to call - // actual functions, so these functions do nothing and return -1. In other - // cases, these functions call actual mlock/munlock functions and return it's - // result. - // On Android, page-out is probably acceptable because - // - Smaller RAM on the device. - // - The storage is (usually) solid state thus page-in/out is expected to - // be faster. - // On Linux, in the kernel version >= 2.6.9, user process can mlock. In older - // kernel, it fails if the process is running in user priviledge. - // TODO(horo): Check in mac that mlock is really necessary. - static int MaybeMLock(const void *addr, size_t len); - - static int MaybeMUnlock(const void *addr, size_t len); - private: DISALLOW_IMPLICIT_CONSTRUCTORS(SystemUtil); }; diff --git a/src/base/system_util_test.cc b/src/base/system_util_test.cc index eb5b302cc..4eee34c00 100644 --- a/src/base/system_util_test.cc +++ b/src/base/system_util_test.cc @@ -239,39 +239,4 @@ TEST_F(SystemUtilTest, GetOSVersionStringTestForAndroid) { } #endif // OS_ANDROID -#ifdef OS_WIN -TEST_F(SystemUtilTest, WindowsMaybeMLockTest) { - size_t data_len = 32; - void *addr = malloc(data_len); - EXPECT_EQ(-1, SystemUtil::MaybeMLock(addr, data_len)); - EXPECT_EQ(-1, SystemUtil::MaybeMUnlock(addr, data_len)); - free(addr); -} -#endif // OS_WIN - -#ifdef OS_MACOSX -TEST_F(SystemUtilTest, MacMaybeMLockTest) { - size_t data_len = 32; - void *addr = malloc(data_len); - EXPECT_EQ(0, SystemUtil::MaybeMLock(addr, data_len)); - EXPECT_EQ(0, SystemUtil::MaybeMUnlock(addr, data_len)); - free(addr); -} -#endif // OS_MACOSX - -TEST_F(SystemUtilTest, LinuxMaybeMLockTest) { - size_t data_len = 32; - void *addr = malloc(data_len); -#ifdef OS_LINUX -#if defined(OS_ANDROID) || defined(__native_client__) - EXPECT_EQ(-1, SystemUtil::MaybeMLock(addr, data_len)); - EXPECT_EQ(-1, SystemUtil::MaybeMUnlock(addr, data_len)); -#else - EXPECT_EQ(0, SystemUtil::MaybeMLock(addr, data_len)); - EXPECT_EQ(0, SystemUtil::MaybeMUnlock(addr, data_len)); -#endif // defined(OS_ANDROID) || defined(__native_client__) -#endif // OS_LINUX - free(addr); -} - } // namespace mozc diff --git a/src/dictionary/system/system_dictionary.cc b/src/dictionary/system/system_dictionary.cc index 85de1f09a..25e54ed2a 100644 --- a/src/dictionary/system/system_dictionary.cc +++ b/src/dictionary/system/system_dictionary.cc @@ -56,9 +56,9 @@ #include #include "base/logging.h" +#include "base/mmap.h" #include "base/port.h" #include "base/string_piece.h" -#include "base/system_util.h" #include "base/util.h" #include "dictionary/dictionary_token.h" #include "dictionary/file/dictionary_file.h" @@ -598,7 +598,7 @@ SystemDictionary *SystemDictionary::Builder::Build() { // has the priviledge to mlock. // Note that we don't munlock the space because it's always better to keep // the singleton system dictionary paged in as long as the process runs. - SystemUtil::MaybeMLock(spec_->ptr, spec_->len); + Mmap::MaybeMLock(spec_->ptr, spec_->len); if (!instance->dictionary_file_->OpenFromImage(spec_->ptr, spec_->len)) { LOG(ERROR) << "Failed to open system dictionary image"; return NULL; diff --git a/src/dictionary/system/value_dictionary.cc b/src/dictionary/system/value_dictionary.cc index 27ae95e16..56ce1070d 100644 --- a/src/dictionary/system/value_dictionary.cc +++ b/src/dictionary/system/value_dictionary.cc @@ -34,9 +34,9 @@ #include #include "base/logging.h" +#include "base/mmap.h" #include "base/port.h" #include "base/string_piece.h" -#include "base/system_util.h" #include "dictionary/dictionary_token.h" #include "dictionary/file/dictionary_file.h" #include "dictionary/pos_matcher.h" @@ -80,7 +80,7 @@ ValueDictionary *ValueDictionary::CreateValueDictionaryFromImage( // has the priviledge to mlock. // Note that we don't munlock the space because it's always better to keep // the singleton system dictionary paged in as long as the process runs. - SystemUtil::MaybeMLock(ptr, len); + Mmap::MaybeMLock(ptr, len); scoped_ptr instance(new ValueDictionary(pos_matcher)); DCHECK(instance.get()); if (!instance->dictionary_file_->OpenFromImage(ptr, len)) { diff --git a/src/mozc_version_template.txt b/src/mozc_version_template.txt index 01e955d91..ae3c96ab2 100644 --- a/src/mozc_version_template.txt +++ b/src/mozc_version_template.txt @@ -1,6 +1,6 @@ MAJOR=2 MINOR=17 -BUILD=2097 +BUILD=2098 REVISION=102 # NACL_DICTIONARY_VERSION is the target version of the system dictionary to be # downloaded by NaCl Mozc.