This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 5bf1f73 env: add common File base class
5bf1f73 is described below
commit 5bf1f73f71175a7d7903c4f1f991f94d615258f9
Author: Adar Dembo <[email protected]>
AuthorDate: Wed Dec 18 23:32:52 2019 -0800
env: add common File base class
It does very little. The key is the common destructor, which simplifies the
(eventual) file cache that can manage multiple file abstractions: when
evicting from the underlying LRU cache, we can cast the evicted pointer to a
File and trust that the appropriate destructor will be invoked.
Change-Id: Ia929234ac8e935a5f1730a3ac575bfb4d1715861
Reviewed-on: http://gerrit.cloudera.org:8080/14987
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Adar Dembo <[email protected]>
---
src/kudu/util/env.cc | 11 +----------
src/kudu/util/env.h | 49 ++++++++++++++++------------------------------
src/kudu/util/env_posix.cc | 12 ++++++------
3 files changed, 24 insertions(+), 48 deletions(-)
diff --git a/src/kudu/util/env.cc b/src/kudu/util/env.cc
index 90755e0..3bc5ee7 100644
--- a/src/kudu/util/env.cc
+++ b/src/kudu/util/env.cc
@@ -18,16 +18,7 @@ namespace kudu {
Env::~Env() {
}
-SequentialFile::~SequentialFile() {
-}
-
-RandomAccessFile::~RandomAccessFile() {
-}
-
-WritableFile::~WritableFile() {
-}
-
-RWFile::~RWFile() {
+File::~File() {
}
FileLock::~FileLock() {
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 38d0356..90fb7aa 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -368,16 +368,22 @@ class Env {
static const char* const kInjectedFailureStatusMsg;
private:
- // No copying allowed
- Env(const Env&);
- void operator=(const Env&);
+ DISALLOW_COPY_AND_ASSIGN(Env);
+};
+
+// A bare-bones abstraction common to all file implementations.
+class File {
+ public:
+ virtual ~File();
+
+ // Returns the filename provided at construction time.
+ virtual const std::string& filename() const = 0;
};
// A file abstraction for reading sequentially through a file
-class SequentialFile {
+class SequentialFile : public File {
public:
SequentialFile() { }
- virtual ~SequentialFile();
// Read up to "result.size" bytes from the file.
// Sets "result.data" to the data that was read.
@@ -396,9 +402,6 @@ class SequentialFile {
//
// REQUIRES: External synchronization
virtual Status Skip(uint64_t n) = 0;
-
- // Returns the filename provided when the SequentialFile was constructed.
- virtual const std::string& filename() const = 0;
};
// A file abstraction for randomly reading the contents of a file.
@@ -407,10 +410,9 @@ class SequentialFile {
// implementations must ensure that any mutable state changes brought on by
// instance destruction and recreation (i.e. triggered by cache eviction and
// reloading events) do not affect correctness.
-class RandomAccessFile {
+class RandomAccessFile : public File {
public:
RandomAccessFile() { }
- virtual ~RandomAccessFile();
// Read "result.size" bytes from the file starting at "offset".
// Copies the resulting data into "result.data".
@@ -440,9 +442,6 @@ class RandomAccessFile {
// Returns the size of the file
virtual Status Size(uint64_t *size) const = 0;
- // Returns the filename provided when the RandomAccessFile was constructed.
- virtual const std::string& filename() const = 0;
-
// Returns the approximate memory usage of this RandomAccessFile including
// the object itself.
virtual size_t memory_footprint() const = 0;
@@ -469,7 +468,7 @@ struct RandomAccessFileOptions {
// A file abstraction for sequential writing. The implementation
// must provide buffering since callers may append small fragments
// at a time to the file.
-class WritableFile {
+class WritableFile : public File {
public:
enum FlushMode {
FLUSH_SYNC,
@@ -477,7 +476,6 @@ class WritableFile {
};
WritableFile() { }
- virtual ~WritableFile();
virtual Status Append(const Slice& data) = 0;
@@ -512,13 +510,8 @@ class WritableFile {
virtual uint64_t Size() const = 0;
- // Returns the filename provided when the WritableFile was constructed.
- virtual const std::string& filename() const = 0;
-
private:
- // No copying allowed
- WritableFile(const WritableFile&);
- void operator=(const WritableFile&);
+ DISALLOW_COPY_AND_ASSIGN(WritableFile);
};
// Creation-time options for RWFile
@@ -547,17 +540,14 @@ struct RWFileOptions {
// implementations must ensure that any mutable state changes brought on by
// instance destruction and recreation (i.e. triggered by cache eviction and
// reloading events) do not affect correctness.
-class RWFile {
+class RWFile : public File {
public:
enum FlushMode {
FLUSH_SYNC,
FLUSH_ASYNC
};
- RWFile() {
- }
-
- virtual ~RWFile();
+ RWFile() { }
// Read "result.size" bytes from the file starting at "offset".
// Copies the resulting data into "result.data".
@@ -659,9 +649,6 @@ class RWFile {
typedef std::map<uint64_t, uint64_t> ExtentMap;
virtual Status GetExtentMap(ExtentMap* out) const = 0;
- // Returns the filename provided when the RWFile was constructed.
- virtual const std::string& filename() const = 0;
-
private:
DISALLOW_COPY_AND_ASSIGN(RWFile);
};
@@ -672,9 +659,7 @@ class FileLock {
FileLock() { }
virtual ~FileLock();
private:
- // No copying allowed
- FileLock(const FileLock&);
- void operator=(const FileLock&);
+ DISALLOW_COPY_AND_ASSIGN(FileLock);
};
// A utility routine: write "data" to the named file.
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index df29a32..bca9f88 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -524,6 +524,7 @@ const char*
ResourceLimitTypeToString(Env::ResourceLimitType t) {
return "running threads per effective uid";
default: LOG(FATAL) << "Unknown resource limit type";
}
+ __builtin_unreachable();
}
int ResourceLimitTypeToUnixRlimit(Env::ResourceLimitType t) {
@@ -532,6 +533,7 @@ int ResourceLimitTypeToUnixRlimit(Env::ResourceLimitType t)
{
case Env::ResourceLimitType::RUNNING_THREADS_PER_EUID: return RLIMIT_NPROC;
default: LOG(FATAL) << "Unknown resource limit type: " << t;
}
+ __builtin_unreachable();
}
#ifdef __APPLE__
@@ -543,6 +545,7 @@ const char*
ResourceLimitTypeToMacosRlimit(Env::ResourceLimitType t) {
return "kern.maxprocperuid";
default: LOG(FATAL) << "Unknown resource limit type: " << t;
}
+ __builtin_unreachable();
}
#endif
@@ -554,7 +557,7 @@ class PosixSequentialFile: public SequentialFile {
public:
PosixSequentialFile(string fname, FILE* f)
: filename_(std::move(fname)), file_(f) {}
- virtual ~PosixSequentialFile() {
+ ~PosixSequentialFile() {
int err;
RETRY_ON_EINTR(err, fclose(file_));
if (PREDICT_FALSE(err != 0)) {
@@ -603,7 +606,7 @@ class PosixRandomAccessFile: public RandomAccessFile {
public:
PosixRandomAccessFile(string fname, int fd)
: filename_(std::move(fname)), fd_(fd) {}
- virtual ~PosixRandomAccessFile() {
+ ~PosixRandomAccessFile() {
int err;
RETRY_ON_EINTR(err, close(fd_));
if (PREDICT_FALSE(err != 0)) {
@@ -1070,8 +1073,7 @@ class PosixFileLock : public FileLock {
class PosixEnv : public Env {
public:
- PosixEnv();
- virtual ~PosixEnv() {
+ ~PosixEnv() {
fprintf(stderr, "Destroying Env::Default()\n");
exit(1);
}
@@ -1838,8 +1840,6 @@ class PosixEnv : public Env {
}
};
-PosixEnv::PosixEnv() {}
-
} // namespace
static pthread_once_t once = PTHREAD_ONCE_INIT;
