Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to upstream (1.22+) #25

Closed
laanwj opened this issue Nov 6, 2019 · 7 comments
Closed

Update to upstream (1.22+) #25

laanwj opened this issue Nov 6, 2019 · 7 comments

Comments

@laanwj
Copy link
Member

laanwj commented Nov 6, 2019

There have been quite a lot of updates upstream since the last pull from there.

But I'm sure there's other changes that make sense for us in the mean time

Specifically: google/leveldb@a53934a...master

Unfortunately this is far from a clean merge:

 .gitignore             | Unmerged
 build_detect_platform  | Unmerged
 db/c.cc                | Unmerged
 db/db_impl.cc          | Unmerged
 db/leveldbutil.cc      | Unmerged
 db/repair.cc           | Unmerged
 include/leveldb/env.h  | Unmerged
 port/atomic_pointer.h  | Unmerged
 port/port.h            | Unmerged
 port/port_posix.cc     | Unmerged
 port/port_posix.h      | Unmerged
 port/port_posix_sse.cc | Unmerged
 util/env_posix.cc      | Unmerged
 util/logging.cc        | Unmerged
Full list of conflicts
diff --cc .gitignore
index 71d87a4eeb60b9599e6a7a23d61a659c7befa553,c4b242534fb45faa8e608f4d5234469439dd1460..0000000000000000000000000000000000000000
--- a/.gitignore
+++ b/.gitignore
@@@ -1,13 -1,8 +1,34 @@@
++<<<<<<< HEAD
 +build_config.mk
 +*.a
 +*.o
 +*.dylib*
 +*.so
 +*.so.*
 +*_test
 +db_bench
 +leveldbutil
 +Release
 +Debug
 +Benchmark
 +vs2010.*
++||||||| merged common ancestors
++build_config.mk
++*.a
++*.o
++*.dylib*
++*.so
++*.so.*
++*_test
++db_bench
++leveldbutil
++=======
+ # Editors.
+ *.sw*
+ .vscode
+ .DS_Store
+ 
+ # Build directory.
+ build/
+ out/
++>>>>>>> master
diff --cc db/c.cc
index b23e3dcc9d06a1d854152b245de6918297026072,3a492f9ac558381d676ceb1249c20346c91eda7d..0000000000000000000000000000000000000000
--- a/db/c.cc
+++ b/db/c.cc
@@@ -4,10 -4,9 +4,19 @@@
  
  #include "leveldb/c.h"
  
++<<<<<<< HEAD
 +#include <stdlib.h>
 +#ifndef WIN32
 +#include <unistd.h>
 +#endif
++||||||| merged common ancestors
++#include <stdlib.h>
++#include <unistd.h>
++=======
+ #include <cstdint>
+ #include <cstdlib>
+ 
++>>>>>>> master
  #include "leveldb/cache.h"
  #include "leveldb/comparator.h"
  #include "leveldb/db.h"
diff --cc db/db_impl.cc
index 3bb58e560aa7c099c7937d226315d1ed46273d43,95e2bb4107d03a77c58c72af3659aafabeb3b4db..0000000000000000000000000000000000000000
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@@ -409,12 -424,11 +424,19 @@@ Status DBImpl::RecoverLogFile(uint64_t 
    Slice record;
    WriteBatch batch;
    int compactions = 0;
-   MemTable* mem = NULL;
-   while (reader.ReadRecord(&record, &scratch) &&
-          status.ok()) {
+   MemTable* mem = nullptr;
+   while (reader.ReadRecord(&record, &scratch) && status.ok()) {
      if (record.size() < 12) {
++<<<<<<< HEAD
 +      reporter.Corruption(
 +          record.size(), Status::Corruption("log record too small", fname));
++||||||| merged common ancestors
++      reporter.Corruption(
++          record.size(), Status::Corruption("log record too small"));
++=======
+       reporter.Corruption(record.size(),
+                           Status::Corruption("log record too small"));
++>>>>>>> master
        continue;
      }
      WriteBatchInternal::SetContents(&batch, record);
diff --cc db/leveldbutil.cc
index d06d64d640b19f60a37571dcb887be5f36ce1514,55cdcc5772236a042a63a25fd260fcc1e412ec69..0000000000000000000000000000000000000000
--- a/db/leveldbutil.cc
+++ b/db/leveldbutil.cc
@@@ -16,10 -17,9 +17,20 @@@ class StdoutPrinter : public WritableFi
      fwrite(data.data(), 1, data.size(), stdout);
      return Status::OK();
    }
++<<<<<<< HEAD
 +  virtual Status Close() { return Status::OK(); }
 +  virtual Status Flush() { return Status::OK(); }
 +  virtual Status Sync() { return Status::OK(); }
 +  virtual std::string GetName() const { return "[stdout]"; }
++||||||| merged common ancestors
++  virtual Status Close() { return Status::OK(); }
++  virtual Status Flush() { return Status::OK(); }
++  virtual Status Sync() { return Status::OK(); }
++=======
+   Status Close() override { return Status::OK(); }
+   Status Flush() override { return Status::OK(); }
+   Status Sync() override { return Status::OK(); }
++>>>>>>> master
  };
  
  bool HandleDumpCommand(Env* env, char** files, int num) {
diff --cc db/repair.cc
index 7281e3d3457fe3a088a18472c25b699b986b42d5,d9d12ba556f0e78bd50218c4f9b06c9283fdd0a8..0000000000000000000000000000000000000000
--- a/db/repair.cc
+++ b/db/repair.cc
@@@ -202,8 -182,8 +182,16 @@@ class Repairer 
      int counter = 0;
      while (reader.ReadRecord(&record, &scratch)) {
        if (record.size() < 12) {
++<<<<<<< HEAD
 +        reporter.Corruption(
 +            record.size(), Status::Corruption("log record too small", logname));
++||||||| merged common ancestors
++        reporter.Corruption(
++            record.size(), Status::Corruption("log record too small"));
++=======
+         reporter.Corruption(record.size(),
+                             Status::Corruption("log record too small"));
++>>>>>>> master
          continue;
        }
        WriteBatchInternal::SetContents(&batch, record);
diff --cc include/leveldb/env.h
index 275d441eaeee49aa3418b0f672d03bd61d8bab6c,112fe964fb1cb2ce3e208ba64b92a82147279266..0000000000000000000000000000000000000000
--- a/include/leveldb/env.h
+++ b/include/leveldb/env.h
@@@ -190,14 -217,6 +217,23 @@@ class LEVELDB_EXPORT SequentialFile 
    //
    // REQUIRES: External synchronization
    virtual Status Skip(uint64_t n) = 0;
++<<<<<<< HEAD
 +
 +  // Get a name for the file, only for error reporting
 +  virtual std::string GetName() const = 0;
 +
 + private:
 +  // No copying allowed
 +  SequentialFile(const SequentialFile&);
 +  void operator=(const SequentialFile&);
++||||||| merged common ancestors
++
++ private:
++  // No copying allowed
++  SequentialFile(const SequentialFile&);
++  void operator=(const SequentialFile&);
++=======
++>>>>>>> master
  };
  
  // A file abstraction for randomly reading the contents of a file.
@@@ -217,14 -240,6 +257,23 @@@ class LEVELDB_EXPORT RandomAccessFile 
    // Safe for concurrent use by multiple threads.
    virtual Status Read(uint64_t offset, size_t n, Slice* result,
                        char* scratch) const = 0;
++<<<<<<< HEAD
 +
 +  // Get a name for the file, only for error reporting
 +  virtual std::string GetName() const = 0;
 +
 + private:
 +  // No copying allowed
 +  RandomAccessFile(const RandomAccessFile&);
 +  void operator=(const RandomAccessFile&);
++||||||| merged common ancestors
++
++ private:
++  // No copying allowed
++  RandomAccessFile(const RandomAccessFile&);
++  void operator=(const RandomAccessFile&);
++=======
++>>>>>>> master
  };
  
  // A file abstraction for sequential writing.  The implementation
@@@ -239,14 -258,6 +292,23 @@@ class LEVELDB_EXPORT WritableFile 
    virtual Status Close() = 0;
    virtual Status Flush() = 0;
    virtual Status Sync() = 0;
++<<<<<<< HEAD
 +
 +  // Get a name for the file, only for error reporting
 +  virtual std::string GetName() const = 0;
 +
 + private:
 +  // No copying allowed
 +  WritableFile(const WritableFile&);
 +  void operator=(const WritableFile&);
++||||||| merged common ancestors
++
++ private:
++  // No copying allowed
++  WritableFile(const WritableFile&);
++  void operator=(const WritableFile&);
++=======
++>>>>>>> master
  };
  
  // An interface for writing log messages.
diff --cc port/port.h
index 4baafa8e22fd290cfd73ad4daf0b5245e0d109c1,4b247f74f9f5b848a5cc3225c23be0dd7726b53e..0000000000000000000000000000000000000000
--- a/port/port.h
+++ b/port/port.h
@@@ -10,12 -10,10 +10,18 @@@
  // Include the appropriate platform specific file below.  If you are
  // porting to a new platform, see "port_example.h" for documentation
  // of what the new port_<platform>.h file must provide.
- #if defined(LEVELDB_PLATFORM_POSIX)
- #  include "port/port_posix.h"
+ #if defined(LEVELDB_PLATFORM_POSIX) || defined(LEVELDB_PLATFORM_WINDOWS)
+ #include "port/port_stdcxx.h"
  #elif defined(LEVELDB_PLATFORM_CHROMIUM)
++<<<<<<< HEAD
 +#  include "port/port_chromium.h"
 +#elif defined(LEVELDB_PLATFORM_WINDOWS)
 +#  include "port/port_win.h"
++||||||| merged common ancestors
++#  include "port/port_chromium.h"
++=======
+ #include "port/port_chromium.h"
++>>>>>>> master
  #endif
  
  #endif  // STORAGE_LEVELDB_PORT_PORT_H_
diff --cc util/env_posix.cc
index f77918313eda73b3b31e58d4ea153db038508bf1,00ca9aedef58b2bd6e2439cc52dee37fffb67c97..0000000000000000000000000000000000000000
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@@ -1,15 -1,10 +1,11 @@@
  // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
  // Use of this source code is governed by a BSD-style license that can be
  // found in the LICENSE file. See the AUTHORS file for names of contributors.
 +#if !defined(LEVELDB_PLATFORM_WINDOWS)
  
  #include <dirent.h>
- #include <errno.h>
  #include <fcntl.h>
  #include <pthread.h>
- #include <stdio.h>
- #include <stdlib.h>
- #include <string.h>
  #include <sys/mman.h>
  #include <sys/resource.h>
  #include <sys/stat.h>
@@@ -121,26 -134,29 +135,37 @@@ class PosixSequentialFile final : publi
      }
      return Status::OK();
    }
++<<<<<<< HEAD
 +
 +  virtual std::string GetName() const { return filename_; }
 +};
++||||||| merged common ancestors
++};
++=======
++>>>>>>> master
  
- // pread() based random-access
- class PosixRandomAccessFile: public RandomAccessFile {
   private:
-   std::string filename_;
-   bool temporary_fd_;  // If true, fd_ is -1 and we open on every read.
-   int fd_;
-   Limiter* limiter_;
+   const int fd_;
+   const std::string filename_;
+ };
  
+ // Implements random read access in a file using pread().
+ //
+ // Instances of this class are thread-safe, as required by the RandomAccessFile
+ // API. Instances are immutable and Read() only calls thread-safe library
+ // functions.
+ class PosixRandomAccessFile final : public RandomAccessFile {
   public:
-   PosixRandomAccessFile(const std::string& fname, int fd, Limiter* limiter)
-       : filename_(fname), fd_(fd), limiter_(limiter) {
-     temporary_fd_ = !limiter->Acquire();
-     if (temporary_fd_) {
-       // Open file on every access.
-       close(fd_);
-       fd_ = -1;
+   // The new instance takes ownership of |fd|. |fd_limiter| must outlive this
+   // instance, and will be used to determine if .
+   PosixRandomAccessFile(std::string filename, int fd, Limiter* fd_limiter)
+       : has_permanent_fd_(fd_limiter->Acquire()),
+         fd_(has_permanent_fd_ ? fd : -1),
+         fd_limiter_(fd_limiter),
+         filename_(std::move(filename)) {
+     if (!has_permanent_fd_) {
+       assert(fd_ == -1);
+       ::close(fd);  // The file will be opened on every read.
      }
    }
  
@@@ -161,72 -178,87 +187,103 @@@
        }
      }
  
-     Status s;
-     ssize_t r = pread(fd, scratch, n, static_cast<off_t>(offset));
-     *result = Slice(scratch, (r < 0) ? 0 : r);
-     if (r < 0) {
-       // An error: return a non-ok status
-       s = IOError(filename_, errno);
+     assert(fd != -1);
+ 
+     Status status;
+     ssize_t read_size = ::pread(fd, scratch, n, static_cast<off_t>(offset));
+     *result = Slice(scratch, (read_size < 0) ? 0 : read_size);
+     if (read_size < 0) {
+       // An error: return a non-ok status.
+       status = PosixError(filename_, errno);
      }
-     if (temporary_fd_) {
+     if (!has_permanent_fd_) {
        // Close the temporary file descriptor opened earlier.
-       close(fd);
+       assert(fd != fd_);
+       ::close(fd);
      }
-     return s;
+     return status;
    }
++<<<<<<< HEAD
 +
 +  virtual std::string GetName() const { return filename_; }
 +};
++||||||| merged common ancestors
++};
++=======
++>>>>>>> master
  
- // mmap() based random-access
- class PosixMmapReadableFile: public RandomAccessFile {
   private:
-   std::string filename_;
-   void* mmapped_region_;
-   size_t length_;
-   Limiter* limiter_;
+   const bool has_permanent_fd_;  // If false, the file is opened on every read.
+   const int fd_;                 // -1 if has_permanent_fd_ is false.
+   Limiter* const fd_limiter_;
+   const std::string filename_;
+ };
  
+ // Implements random read access in a file using mmap().
+ //
+ // Instances of this class are thread-safe, as required by the RandomAccessFile
+ // API. Instances are immutable and Read() only calls thread-safe library
+ // functions.
+ class PosixMmapReadableFile final : public RandomAccessFile {
   public:
-   // base[0,length-1] contains the mmapped contents of the file.
-   PosixMmapReadableFile(const std::string& fname, void* base, size_t length,
-                         Limiter* limiter)
-       : filename_(fname), mmapped_region_(base), length_(length),
-         limiter_(limiter) {
-   }
- 
-   virtual ~PosixMmapReadableFile() {
-     munmap(mmapped_region_, length_);
-     limiter_->Release();
-   }
- 
-   virtual Status Read(uint64_t offset, size_t n, Slice* result,
-                       char* scratch) const {
-     Status s;
+   // mmap_base[0, length-1] points to the memory-mapped contents of the file. It
+   // must be the result of a successful call to mmap(). This instances takes
+   // over the ownership of the region.
+   //
+   // |mmap_limiter| must outlive this instance. The caller must have already
+   // aquired the right to use one mmap region, which will be released when this
+   // instance is destroyed.
+   PosixMmapReadableFile(std::string filename, char* mmap_base, size_t length,
+                         Limiter* mmap_limiter)
+       : mmap_base_(mmap_base),
+         length_(length),
+         mmap_limiter_(mmap_limiter),
+         filename_(std::move(filename)) {}
+ 
+   ~PosixMmapReadableFile() override {
+     ::munmap(static_cast<void*>(mmap_base_), length_);
+     mmap_limiter_->Release();
+   }
+ 
+   Status Read(uint64_t offset, size_t n, Slice* result,
+               char* scratch) const override {
      if (offset + n > length_) {
        *result = Slice();
-       s = IOError(filename_, EINVAL);
-     } else {
-       *result = Slice(reinterpret_cast<char*>(mmapped_region_) + offset, n);
+       return PosixError(filename_, EINVAL);
      }
-     return s;
+ 
+     *result = Slice(mmap_base_ + offset, n);
+     return Status::OK();
    }
++<<<<<<< HEAD
 +
 +  virtual std::string GetName() const { return filename_; }
 +};
++||||||| merged common ancestors
++};
++=======
++>>>>>>> master
  
- class PosixWritableFile : public WritableFile {
   private:
-   std::string filename_;
-   FILE* file_;
+   char* const mmap_base_;
+   const size_t length_;
+   Limiter* const mmap_limiter_;
+   const std::string filename_;
+ };
  
+ class PosixWritableFile final : public WritableFile {
   public:
-   PosixWritableFile(const std::string& fname, FILE* f)
-       : filename_(fname), file_(f) { }
- 
-   ~PosixWritableFile() {
-     if (file_ != NULL) {
+   PosixWritableFile(std::string filename, int fd)
+       : pos_(0),
+         fd_(fd),
+         is_manifest_(IsManifest(filename)),
+         filename_(std::move(filename)),
+         dirname_(Dirname(filename_)) {}
+ 
+   ~PosixWritableFile() override {
+     if (fd_ >= 0) {
        // Ignoring any potential errors
-       fclose(file_);
+       Close();
      }
    }
  
@@@ -255,81 -345,146 +370,178 @@@
    }
  
    Status SyncDirIfManifest() {
-     const char* f = filename_.c_str();
-     const char* sep = strrchr(f, '/');
-     Slice basename;
-     std::string dir;
-     if (sep == NULL) {
-       dir = ".";
-       basename = f;
+     Status status;
+     if (!is_manifest_) {
+       return status;
+     }
+ 
+     int fd = ::open(dirname_.c_str(), O_RDONLY | kOpenBaseFlags);
+     if (fd < 0) {
+       status = PosixError(dirname_, errno);
      } else {
-       dir = std::string(f, sep - f);
-       basename = sep + 1;
+       status = SyncFd(fd, dirname_);
+       ::close(fd);
+     }
+     return status;
+   }
+ 
+   // Ensures that all the caches associated with the given file descriptor's
+   // data are flushed all the way to durable media, and can withstand power
+   // failures.
+   //
+   // The path argument is only used to populate the description string in the
+   // returned Status if an error occurs.
+   static Status SyncFd(int fd, const std::string& fd_path) {
+ #if HAVE_FULLFSYNC
+     // On macOS and iOS, fsync() doesn't guarantee durability past power
+     // failures. fcntl(F_FULLFSYNC) is required for that purpose. Some
+     // filesystems don't support fcntl(F_FULLFSYNC), and require a fallback to
+     // fsync().
+     if (::fcntl(fd, F_FULLFSYNC) == 0) {
+       return Status::OK();
      }
++<<<<<<< HEAD
 +    Status s;
 +    if (basename.starts_with("MANIFEST")) {
 +      int fd = open(dir.c_str(), O_RDONLY);
 +      if (fd < 0) {
 +        s = IOError(dir, errno);
 +      } else {
 +        if (fsync(fd) < 0 && errno != EINVAL) {
 +          s = IOError(dir, errno);
 +        }
 +        close(fd);
 +      }
++||||||| merged common ancestors
++    Status s;
++    if (basename.starts_with("MANIFEST")) {
++      int fd = open(dir.c_str(), O_RDONLY);
++      if (fd < 0) {
++        s = IOError(dir, errno);
++      } else {
++        if (fsync(fd) < 0) {
++          s = IOError(dir, errno);
++        }
++        close(fd);
++      }
++=======
+ #endif  // HAVE_FULLFSYNC
+ 
+ #if HAVE_FDATASYNC
+     bool sync_success = ::fdatasync(fd) == 0;
+ #else
+     bool sync_success = ::fsync(fd) == 0;
+ #endif  // HAVE_FDATASYNC
+ 
+     if (sync_success) {
+       return Status::OK();
++>>>>>>> master
      }
-     return s;
+     return PosixError(fd_path, errno);
    }
  
-   virtual Status Sync() {
-     // Ensure new files referred to by the manifest are in the filesystem.
-     Status s = SyncDirIfManifest();
-     if (!s.ok()) {
-       return s;
+   // Returns the directory name in a path pointing to a file.
+   //
+   // Returns "." if the path does not contain any directory separator.
+   static std::string Dirname(const std::string& filename) {
+     std::string::size_type separator_pos = filename.rfind('/');
+     if (separator_pos == std::string::npos) {
+       return std::string(".");
      }
-     if (fflush_unlocked(file_) != 0 ||
-         fdatasync(fileno(file_)) != 0) {
-       s = Status::IOError(filename_, strerror(errno));
+     // The filename component should not contain a path separator. If it does,
+     // the splitting was done incorrectly.
+     assert(filename.find('/', separator_pos + 1) == std::string::npos);
+ 
+     return filename.substr(0, separator_pos);
+   }
+ 
+   // Extracts the file name from a path pointing to a file.
+   //
+   // The returned Slice points to |filename|'s data buffer, so it is only valid
+   // while |filename| is alive and unchanged.
+   static Slice Basename(const std::string& filename) {
+     std::string::size_type separator_pos = filename.rfind('/');
+     if (separator_pos == std::string::npos) {
+       return Slice(filename);
      }
-     return s;
+     // The filename component should not contain a path separator. If it does,
+     // the splitting was done incorrectly.
+     assert(filename.find('/', separator_pos + 1) == std::string::npos);
+ 
+     return Slice(filename.data() + separator_pos + 1,
+                  filename.length() - separator_pos - 1);
    }
  
+   // True if the given file is a manifest file.
+   static bool IsManifest(const std::string& filename) {
+     return Basename(filename).starts_with("MANIFEST");
+   }
++<<<<<<< HEAD
++
 +  virtual std::string GetName() const { return filename_; }
++||||||| merged common ancestors
++=======
+ 
+   // buf_[0, pos_ - 1] contains data to be written to fd_.
+   char buf_[kWritableFileBufferSize];
+   size_t pos_;
+   int fd_;
+ 
+   const bool is_manifest_;  // True if the file's name starts with MANIFEST.
+   const std::string filename_;
+   const std::string dirname_;  // The directory of filename_.
++>>>>>>> master
  };
  
- static int LockOrUnlock(int fd, bool lock) {
+ int LockOrUnlock(int fd, bool lock) {
    errno = 0;
-   struct flock f;
-   memset(&f, 0, sizeof(f));
-   f.l_type = (lock ? F_WRLCK : F_UNLCK);
-   f.l_whence = SEEK_SET;
-   f.l_start = 0;
-   f.l_len = 0;        // Lock/unlock entire file
-   return fcntl(fd, F_SETLK, &f);
+   struct ::flock file_lock_info;
+   std::memset(&file_lock_info, 0, sizeof(file_lock_info));
+   file_lock_info.l_type = (lock ? F_WRLCK : F_UNLCK);
+   file_lock_info.l_whence = SEEK_SET;
+   file_lock_info.l_start = 0;
+   file_lock_info.l_len = 0;  // Lock/unlock entire file.
+   return ::fcntl(fd, F_SETLK, &file_lock_info);
  }
  
+ // Instances are thread-safe because they are immutable.
  class PosixFileLock : public FileLock {
   public:
-   int fd_;
-   std::string name_;
+   PosixFileLock(int fd, std::string filename)
+       : fd_(fd), filename_(std::move(filename)) {}
+ 
+   int fd() const { return fd_; }
+   const std::string& filename() const { return filename_; }
+ 
+  private:
+   const int fd_;
+   const std::string filename_;
  };
  
- // Set of locked files.  We keep a separate set instead of just
- // relying on fcntrl(F_SETLK) since fcntl(F_SETLK) does not provide
- // any protection against multiple uses from the same process.
+ // Tracks the files locked by PosixEnv::LockFile().
+ //
+ // We maintain a separate set instead of relying on fcntl(F_SETLK) because
+ // fcntl(F_SETLK) does not provide any protection against multiple uses from the
+ // same process.
+ //
+ // Instances are thread-safe because all member data is guarded by a mutex.
  class PosixLockTable {
-  private:
-   port::Mutex mu_;
-   std::set<std::string> locked_files_;
   public:
-   bool Insert(const std::string& fname) {
-     MutexLock l(&mu_);
-     return locked_files_.insert(fname).second;
-   }
-   void Remove(const std::string& fname) {
-     MutexLock l(&mu_);
+   bool Insert(const std::string& fname) LOCKS_EXCLUDED(mu_) {
+     mu_.Lock();
+     bool succeeded = locked_files_.insert(fname).second;
+     mu_.Unlock();
+     return succeeded;
+   }
+   void Remove(const std::string& fname) LOCKS_EXCLUDED(mu_) {
+     mu_.Lock();
      locked_files_.erase(fname);
+     mu_.Unlock();
    }
+ 
+  private:
+   port::Mutex mu_;
+   std::set<std::string> locked_files_ GUARDED_BY(mu_);
  };
  
  class PosixEnv : public Env {
@@@ -581,80 -750,71 +807,91 @@@
  };
  
  // Return the maximum number of concurrent mmaps.
++<<<<<<< HEAD
 +static int MaxMmaps() {
 +  if (mmap_limit >= 0) {
 +    return mmap_limit;
 +  }
 +  // Up to 4096 mmaps for 64-bit binaries; none for smaller pointer sizes.
 +  mmap_limit = sizeof(void*) >= 8 ? 4096 : 0;
 +  return mmap_limit;
 +}
++||||||| merged common ancestors
++static int MaxMmaps() {
++  if (mmap_limit >= 0) {
++    return mmap_limit;
++  }
++  // Up to 1000 mmaps for 64-bit binaries; none for smaller pointer sizes.
++  mmap_limit = sizeof(void*) >= 8 ? 1000 : 0;
++  return mmap_limit;
++}
++=======
+ int MaxMmaps() { return g_mmap_limit; }
++>>>>>>> master
  
  // Return the maximum number of read-only files to keep open.
- static intptr_t MaxOpenFiles() {
-   if (open_read_only_file_limit >= 0) {
-     return open_read_only_file_limit;
+ int MaxOpenFiles() {
+   if (g_open_read_only_file_limit >= 0) {
+     return g_open_read_only_file_limit;
    }
-   struct rlimit rlim;
-   if (getrlimit(RLIMIT_NOFILE, &rlim)) {
+   struct ::rlimit rlim;
+   if (::getrlimit(RLIMIT_NOFILE, &rlim)) {
      // getrlimit failed, fallback to hard-coded default.
-     open_read_only_file_limit = 50;
+     g_open_read_only_file_limit = 50;
    } else if (rlim.rlim_cur == RLIM_INFINITY) {
-     open_read_only_file_limit = std::numeric_limits<int>::max();
+     g_open_read_only_file_limit = std::numeric_limits<int>::max();
    } else {
      // Allow use of 20% of available file descriptors for read-only files.
-     open_read_only_file_limit = rlim.rlim_cur / 5;
+     g_open_read_only_file_limit = rlim.rlim_cur / 5;
    }
-   return open_read_only_file_limit;
+   return g_open_read_only_file_limit;
  }
  
+ }  // namespace
+ 
  PosixEnv::PosixEnv()
-     : started_bgthread_(false),
-       mmap_limit_(MaxMmaps()),
-       fd_limit_(MaxOpenFiles()) {
-   PthreadCall("mutex_init", pthread_mutex_init(&mu_, NULL));
-   PthreadCall("cvar_init", pthread_cond_init(&bgsignal_, NULL));
- }
+     : background_work_cv_(&background_work_mutex_),
+       started_background_thread_(false),
+       mmap_limiter_(MaxMmaps()),
+       fd_limiter_(MaxOpenFiles()) {}
  
- void PosixEnv::Schedule(void (*function)(void*), void* arg) {
-   PthreadCall("lock", pthread_mutex_lock(&mu_));
+ void PosixEnv::Schedule(
+     void (*background_work_function)(void* background_work_arg),
+     void* background_work_arg) {
+   background_work_mutex_.Lock();
  
-   // Start background thread if necessary
-   if (!started_bgthread_) {
-     started_bgthread_ = true;
-     PthreadCall(
-         "create thread",
-         pthread_create(&bgthread_, NULL,  &PosixEnv::BGThreadWrapper, this));
+   // Start the background thread, if we haven't done so already.
+   if (!started_background_thread_) {
+     started_background_thread_ = true;
+     std::thread background_thread(PosixEnv::BackgroundThreadEntryPoint, this);
+     background_thread.detach();
    }
  
-   // If the queue is currently empty, the background thread may currently be
-   // waiting.
-   if (queue_.empty()) {
-     PthreadCall("signal", pthread_cond_signal(&bgsignal_));
+   // If the queue is empty, the background thread may be waiting for work.
+   if (background_work_queue_.empty()) {
+     background_work_cv_.Signal();
    }
  
-   // Add to priority queue
-   queue_.push_back(BGItem());
-   queue_.back().function = function;
-   queue_.back().arg = arg;
- 
-   PthreadCall("unlock", pthread_mutex_unlock(&mu_));
+   background_work_queue_.emplace(background_work_function, background_work_arg);
+   background_work_mutex_.Unlock();
  }
  
- void PosixEnv::BGThread() {
+ void PosixEnv::BackgroundThreadMain() {
    while (true) {
-     // Wait until there is an item that is ready to run
-     PthreadCall("lock", pthread_mutex_lock(&mu_));
-     while (queue_.empty()) {
-       PthreadCall("wait", pthread_cond_wait(&bgsignal_, &mu_));
+     background_work_mutex_.Lock();
+ 
+     // Wait until there is work to be done.
+     while (background_work_queue_.empty()) {
+       background_work_cv_.Wait();
      }
  
-     void (*function)(void*) = queue_.front().function;
-     void* arg = queue_.front().arg;
-     queue_.pop_front();
+     assert(!background_work_queue_.empty());
+     auto background_work_function = background_work_queue_.front().function;
+     void* background_work_arg = background_work_queue_.front().arg;
+     background_work_queue_.pop();
  
-     PthreadCall("unlock", pthread_mutex_unlock(&mu_));
-     (*function)(arg);
+     background_work_mutex_.Unlock();
+     background_work_function(background_work_arg);
    }
  }
  
diff --cc util/logging.cc
index db6160c8f19929e9517e371ba2295e0d0e38e421,75e9d037d3f9ebeadc9bd78185aa9e100b0e42b8..0000000000000000000000000000000000000000
--- a/util/logging.cc
+++ b/util/logging.cc
@@@ -46,27 -49,36 +49,74 @@@ std::string EscapeString(const Slice& v
  }
  
  bool ConsumeDecimalNumber(Slice* in, uint64_t* val) {
++<<<<<<< HEAD
 +  uint64_t v = 0;
 +  int digits = 0;
 +  while (!in->empty()) {
 +    unsigned char c = (*in)[0];
 +    if (c >= '0' && c <= '9') {
 +      ++digits;
 +      const int delta = (c - '0');
 +      static const uint64_t kMaxUint64 = ~static_cast<uint64_t>(0);
 +      if (v > kMaxUint64/10 ||
 +          (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
 +        // Overflow
 +        return false;
 +      }
 +      v = (v * 10) + delta;
 +      in->remove_prefix(1);
 +    } else {
 +      break;
++||||||| merged common ancestors
++  uint64_t v = 0;
++  int digits = 0;
++  while (!in->empty()) {
++    char c = (*in)[0];
++    if (c >= '0' && c <= '9') {
++      ++digits;
++      const int delta = (c - '0');
++      static const uint64_t kMaxUint64 = ~static_cast<uint64_t>(0);
++      if (v > kMaxUint64/10 ||
++          (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
++        // Overflow
++        return false;
++      }
++      v = (v * 10) + delta;
++      in->remove_prefix(1);
++    } else {
++      break;
++=======
+   // Constants that will be optimized away.
+   constexpr const uint64_t kMaxUint64 = std::numeric_limits<uint64_t>::max();
+   constexpr const char kLastDigitOfMaxUint64 =
+       '0' + static_cast<char>(kMaxUint64 % 10);
+ 
+   uint64_t value = 0;
+ 
+   // reinterpret_cast-ing from char* to uint8_t* to avoid signedness.
+   const uint8_t* start = reinterpret_cast<const uint8_t*>(in->data());
+ 
+   const uint8_t* end = start + in->size();
+   const uint8_t* current = start;
+   for (; current != end; ++current) {
+     const uint8_t ch = *current;
+     if (ch < '0' || ch > '9') break;
+ 
+     // Overflow check.
+     // kMaxUint64 / 10 is also constant and will be optimized away.
+     if (value > kMaxUint64 / 10 ||
+         (value == kMaxUint64 / 10 && ch > kLastDigitOfMaxUint64)) {
+       return false;
++>>>>>>> master
      }
+ 
+     value = (value * 10) + (ch - '0');
    }
-   *val = v;
-   return (digits > 0);
+ 
+   *val = value;
+   const size_t digits_consumed = current - start;
+   in->remove_prefix(digits_consumed);
+   return digits_consumed != 0;
  }
  
  }  // namespace leveldb
* Unmerged path build_detect_platform
* Unmerged path port/atomic_pointer.h
* Unmerged path port/port_posix.cc
* Unmerged path port/port_posix.h
* Unmerged path port/port_posix_sse.cc
@maflcko
Copy link

maflcko commented Nov 6, 2019

They switched to CMake. I don't understand enough of build systems, so I am wondering if this will break our autotools build?

@laanwj
Copy link
Member Author

laanwj commented Nov 6, 2019

Good to know! But I'm fairly sure that doesn't affect bitcoin; we've always built leveldb as part of our own build system in src/Makefile.leveldb.include, explicitly listing the compilation units, and don't invoke a sub-build-system for it.

@maflcko
Copy link

maflcko commented Nov 6, 2019

Oh interesting. I didn't know

@maflcko
Copy link

maflcko commented Nov 6, 2019

Some of the conflicts can be avoided by reverting the fixes we applied locally #10

@laanwj
Copy link
Member Author

laanwj commented Nov 6, 2019

That one definitely causes some unnecessary conflicts.

It seems most of the conflicts are trivial-ish, except some changes in util/env_posix.cc:

  • The manifest-related code here disappeared completely (so need to figure out if the && errno != EINVAL change should be applied elsewhere):
++<<<<<<< HEAD
 +    Status s;
 +    if (basename.starts_with("MANIFEST")) {
 +      int fd = open(dir.c_str(), O_RDONLY);
 +      if (fd < 0) {
 +        s = IOError(dir, errno);
 +      } else {
 +        if (fsync(fd) < 0 && errno != EINVAL) {
 +          s = IOError(dir, errno);
 +        }
 +        close(fd);
 +      }
++||||||| merged common ancestors
++    Status s;
++    if (basename.starts_with("MANIFEST")) {
++      int fd = open(dir.c_str(), O_RDONLY);
++      if (fd < 0) {
++        s = IOError(dir, errno);
++      } else {
++        if (fsync(fd) < 0) {
++          s = IOError(dir, errno);
++        }
++        close(fd);
++      }
++=======
+ #endif  // HAVE_FULLFSYNC
+ 
+ #if HAVE_FDATASYNC
+     bool sync_success = ::fdatasync(fd) == 0;
+ #else
+     bool sync_success = ::fsync(fd) == 0;
+ #endif  // HAVE_FDATASYNC
+ 
+     if (sync_success) {
+       return Status::OK();
++>>>>>>> master

that code moved to this:

  Status SyncDirIfManifest() {
    Status status;
    if (!is_manifest_) {
      return status;
    }

    int fd = ::open(dirname_.c_str(), O_RDONLY | kOpenBaseFlags);
    if (fd < 0) {
      status = PosixError(dirname_, errno);
    } else {
      status = SyncFd(fd, dirname_);
      ::close(fd);
    }
    return status;
  }
  • Also MaxMmaps() implementation moved elsewhere (so need to figure out where to put the 1000->4096 change, if still relevant):
++<<<<<<< HEAD
 +static int MaxMmaps() {
 +  if (mmap_limit >= 0) {
 +    return mmap_limit;
 +  }
 +  // Up to 4096 mmaps for 64-bit binaries; none for smaller pointer sizes.
 +  mmap_limit = sizeof(void*) >= 8 ? 4096 : 0;
 +  return mmap_limit;
 +}
++||||||| merged common ancestors
++static int MaxMmaps() {
++  if (mmap_limit >= 0) {
++    return mmap_limit;
++  }
++  // Up to 1000 mmaps for 64-bit binaries; none for smaller pointer sizes.
++  mmap_limit = sizeof(void*) >= 8 ? 1000 : 0;
++  return mmap_limit;
++}
++=======
+ int MaxMmaps() { return g_mmap_limit; }
++>>>>>>> master

found it

// Up to 1000 mmap regions for 64-bit binaries; none for 32-bit.
constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0;
  • Looks like port_posix (not env_posix) has been replaced with port_stdcxx. This means HasAcceleratedCRC32C must be ported over (edit: oh, this isn't true, only for the header? port_posix is still used for Linux and BSD no, it's not, build_detect_platform is no longer used)

    • Whoooops: "Replace SSE-optimized CRC32C in POSIX port with external library." looks like they removed the hardware-accelerated CRC32C implementation completely. It's been replaced with the google/crc32c library in 5c39524.

@elichai
Copy link

elichai commented Feb 5, 2020

I do not know leveldb's release process. are we sure we want to include those unreleased commits?

@laanwj
Copy link
Member Author

laanwj commented Feb 6, 2020

Leveldb's release process is slow. If you really want to wait for these changes to be in a release, it's going to take a long time.

laanwj added a commit to bitcoin/bitcoin that referenced this issue Feb 10, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan)
8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson)
be23949 build: MSVC changes for leveldb update (Aaron Clauson)
9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan)
402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan)
3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan)
84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan)
7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan)
24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan)
2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan)
6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan)

Pull request description:

  This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829:

  - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future.
  - Thread handling uses C++11, instead of platform specific code.
  - Native windows environment was added. No need to maintain our own hacky one, anymore.
  - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed.

  All changes: google/leveldb@a53934a...0c40829

  Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new

  There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context.

  TODO:
  - [x] Subtree `crc32c`
  - [x] Make linters happy about crc32 subtree
  - [x] Integrate `crc32c` library into build system
  - [x] MSVC build system

ACKs for top commit:
  sipa:
    ACK 677fb8e

Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
sidhujag pushed a commit to syscoin/syscoin that referenced this issue Feb 18, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan)
8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson)
be23949 build: MSVC changes for leveldb update (Aaron Clauson)
9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan)
402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan)
3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan)
84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan)
7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan)
24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan)
2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan)
6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan)

Pull request description:

  This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829:

  - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future.
  - Thread handling uses C++11, instead of platform specific code.
  - Native windows environment was added. No need to maintain our own hacky one, anymore.
  - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed.

  All changes: google/leveldb@a53934a...0c40829

  Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new

  There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context.

  TODO:
  - [x] Subtree `crc32c`
  - [x] Make linters happy about crc32 subtree
  - [x] Integrate `crc32c` library into build system
  - [x] MSVC build system

ACKs for top commit:
  sipa:
    ACK 677fb8e

Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
sidhujag pushed a commit to syscoin-core/syscoin that referenced this issue Nov 10, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan)
8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson)
be23949 build: MSVC changes for leveldb update (Aaron Clauson)
9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan)
402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan)
3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan)
84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan)
7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan)
24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan)
2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan)
6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan)

Pull request description:

  This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829:

  - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future.
  - Thread handling uses C++11, instead of platform specific code.
  - Native windows environment was added. No need to maintain our own hacky one, anymore.
  - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed.

  All changes: google/leveldb@a53934a...0c40829

  Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new

  There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context.

  TODO:
  - [x] Subtree `crc32c`
  - [x] Make linters happy about crc32 subtree
  - [x] Integrate `crc32c` library into build system
  - [x] MSVC build system

ACKs for top commit:
  sipa:
    ACK 677fb8e

Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants