This is an automated email from the ASF dual-hosted git repository.

masaori pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 6979e4c9f6 Cleanup: Span and Store (#10952)
6979e4c9f6 is described below

commit 6979e4c9f6cc1d417798ce90e24bbd9d67e1525c
Author: Masaori Koshiba <masa...@apache.org>
AuthorDate: Tue Jan 9 17:01:05 2024 +0900

    Cleanup: Span and Store (#10952)
---
 include/iocore/cache/Store.h | 74 +++++++-------------------------------------
 src/iocore/cache/Cache.cc    | 18 +++++------
 src/iocore/cache/Store.cc    | 72 ++++++++++++++++++------------------------
 3 files changed, 51 insertions(+), 113 deletions(-)

diff --git a/include/iocore/cache/Store.h b/include/iocore/cache/Store.h
index 15bc3ba302..0b0a762ca8 100644
--- a/include/iocore/cache/Store.h
+++ b/include/iocore/cache/Store.h
@@ -79,29 +79,14 @@ struct Span {
   unsigned hw_sector_size = DEFAULT_HW_SECTOR_SIZE;
   unsigned alignment      = 0;
   span_diskid_t disk_id;
-  int forced_volume_num = -1; ///< Force span in to specific volume.
-private:
-  bool is_mmapable_internal = false;
-
-public:
-  bool file_pathname = false; // the pathname is a file
+  int forced_volume_num = -1;    ///< Force span in to specific volume.
+  bool file_pathname    = false; // the pathname is a file
   // v- used as a magic location for copy constructor.
   // we memcpy everything before this member and do explicit assignment for 
the rest.
   ats_scoped_str pathname;
   ats_scoped_str hash_base_string; ///< Used to seed the stripe assignment 
hash.
-  SLINK(Span, link);
-
-  bool
-  is_mmapable() const
-  {
-    return is_mmapable_internal;
-  }
 
-  void
-  set_mmapable(bool s)
-  {
-    is_mmapable_internal = s;
-  }
+  SLINK(Span, link);
 
   int64_t
   size() const
@@ -109,39 +94,6 @@ public:
     return blocks * STORE_BLOCK_SIZE;
   }
 
-  int64_t
-  total_blocks() const
-  {
-    if (link.next) {
-      return blocks + link.next->total_blocks();
-    } else {
-      return blocks;
-    }
-  }
-
-  Span *
-  nth(unsigned i)
-  {
-    Span *x = this;
-    while (x && i--) {
-      x = x->link.next;
-    }
-    return x;
-  }
-
-  unsigned
-  paths() const
-  {
-    int i = 0;
-    for (const Span *x = this; x; i++, x = x->link.next) {
-      ;
-    }
-
-    return i;
-  }
-
-  /// Duplicate this span and all chained spans.
-  Span *dup();
   int64_t
   end() const
   {
@@ -189,25 +141,23 @@ struct Store {
   void
   extend(unsigned i)
   {
-    if (i > n_disks) {
-      disk = (Span **)ats_realloc(disk, i * sizeof(Span *));
-      for (unsigned j = n_disks; j < i; j++) {
-        disk[j] = nullptr;
+    if (i > n_spans) {
+      spans = static_cast<Span **>(ats_realloc(spans, i * sizeof(Span *)));
+      for (unsigned j = n_spans; j < i; j++) {
+        spans[j] = nullptr;
       }
-      n_disks = i;
+      n_spans = i;
     }
   }
 
   void delete_all();
 
-  Store();
+  Store(){};
   ~Store();
 
-  // The number of disks/paths defined in storage.config
-  unsigned n_disks_in_config = 0;
-  // The number of disks/paths we could actually read and parse.
-  unsigned n_disks = 0;
-  Span **disk      = nullptr;
+  unsigned n_spans_in_config = 0; ///< The number of disks/paths defined in 
storage.config
+  unsigned n_spans           = 0; ///< The number of disks/paths we could 
actually read and parse
+  Span **spans               = nullptr;
 
   Result read_config();
 
diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc
index 7d14385e3d..622cb42a03 100644
--- a/src/iocore/cache/Cache.cc
+++ b/src/iocore/cache/Cache.cc
@@ -302,7 +302,7 @@ CacheProcessor::start_internal(int flags)
   start_done           = 0;
 
   /* Read the config file and create the data structures corresponding to the 
file. */
-  gndisks = theCacheStore.n_disks;
+  gndisks = theCacheStore.n_spans;
   gdisks  = static_cast<CacheDisk **>(ats_malloc(gndisks * sizeof(CacheDisk 
*)));
 
   // Temporaries to carry values between loops
@@ -323,8 +323,8 @@ CacheProcessor::start_internal(int flags)
   /*
    create CacheDisk objects for each span in the configuration file and store 
in gdisks
    */
-  for (unsigned i = 0; i < theCacheStore.n_disks; i++) {
-    Span *sd = theCacheStore.disk[i];
+  for (unsigned i = 0; i < theCacheStore.n_spans; i++) {
+    Span *sd = theCacheStore.spans[i];
     int opts = DEFAULT_CACHE_OPTIONS;
 
     if (!paths[gndisks]) {
@@ -440,15 +440,15 @@ CacheProcessor::start_internal(int flags)
       Warning("unable to open cache disk(s): Cache Disabled\n");
       return -1; // pointless, AFAICT this is ignored.
     }
-  } else if (this->waitForCache() == 3 && static_cast<unsigned int>(gndisks) < 
theCacheStore.n_disks_in_config) {
+  } else if (this->waitForCache() == 3 && static_cast<unsigned int>(gndisks) < 
theCacheStore.n_spans_in_config) {
     CacheProcessor::initialized = CACHE_INIT_FAILED;
     if (cb_after_init) {
       cb_after_init();
     }
     Emergency("Cache initialization failed - only %d out of %d disks were 
valid and all were required.", gndisks,
-              theCacheStore.n_disks_in_config);
-  } else if (this->waitForCache() == 2 && static_cast<unsigned int>(gndisks) < 
theCacheStore.n_disks_in_config) {
-    Warning("Cache initialization incomplete - only %d out of %d disks were 
valid.", gndisks, theCacheStore.n_disks_in_config);
+              theCacheStore.n_spans_in_config);
+  } else if (this->waitForCache() == 2 && static_cast<unsigned int>(gndisks) < 
theCacheStore.n_spans_in_config) {
+    Warning("Cache initialization incomplete - only %d out of %d disks were 
valid.", gndisks, theCacheStore.n_spans_in_config);
   }
 
   // If we got here, we have enough disks to proceed
@@ -500,9 +500,9 @@ CacheProcessor::diskInitialized()
       if (cb_after_init) {
         cb_after_init();
       }
-      Emergency("Cache initialization failed - only %d of %d disks were 
available.", gndisks, theCacheStore.n_disks_in_config);
+      Emergency("Cache initialization failed - only %d of %d disks were 
available.", gndisks, theCacheStore.n_spans_in_config);
     } else if (this->waitForCache() == 2) {
-      Warning("Cache initialization incomplete - only %d of %d disks were 
available.", gndisks, theCacheStore.n_disks_in_config);
+      Warning("Cache initialization incomplete - only %d of %d disks were 
available.", gndisks, theCacheStore.n_spans_in_config);
     }
   }
 
diff --git a/src/iocore/cache/Store.cc b/src/iocore/cache/Store.cc
index 62033e9865..6630f640b7 100644
--- a/src/iocore/cache/Store.cc
+++ b/src/iocore/cache/Store.cc
@@ -21,14 +21,15 @@
   limitations under the License.
  */
 
-#include "tscore/ink_platform.h"
 #include "P_Cache.h"
+
+#include "iocore/cache/Store.h"
+
+#include "tscore/ink_platform.h"
 #include "tscore/Layout.h"
 #include "tscore/Filenames.h"
 #include "tscore/ink_file.h"
-#include "tscore/Tokenizer.h"
 #include "tscore/SimpleTokenizer.h"
-#include "tscore/runroot.h"
 
 #if defined(__linux__)
 #include <linux/major.h>
@@ -78,48 +79,46 @@ span_file_typename(mode_t st_mode)
   }
 }
 
-Store::Store() {}
-
 void
 Store::sort()
 {
-  Span **vec = static_cast<Span **>(alloca(sizeof(Span *) * n_disks));
-  memset(vec, 0, sizeof(Span *) * n_disks);
-  for (unsigned i = 0; i < n_disks; i++) {
-    vec[i]  = disk[i];
-    disk[i] = nullptr;
+  Span **vec = static_cast<Span **>(alloca(sizeof(Span *) * n_spans));
+  memset(vec, 0, sizeof(Span *) * n_spans);
+  for (unsigned i = 0; i < n_spans; i++) {
+    vec[i]   = spans[i];
+    spans[i] = nullptr;
   }
 
   // sort by device
 
   unsigned n = 0;
-  for (unsigned i = 0; i < n_disks; i++) {
+  for (unsigned i = 0; i < n_spans; i++) {
     for (Span *sd = vec[i]; sd; sd = vec[i]) {
       vec[i] = vec[i]->link.next;
       for (unsigned d = 0; d < n; d++) {
-        if (sd->disk_id == disk[d]->disk_id) {
-          sd->link.next = disk[d];
-          disk[d]       = sd;
+        if (sd->disk_id == spans[d]->disk_id) {
+          sd->link.next = spans[d];
+          spans[d]      = sd;
           goto Ldone;
         }
       }
-      disk[n++] = sd;
+      spans[n++] = sd;
     Ldone:;
     }
   }
-  n_disks = n;
+  n_spans = n;
 
   // sort by pathname x offset
 
-  for (unsigned i = 0; i < n_disks; i++) {
+  for (unsigned i = 0; i < n_spans; i++) {
   Lagain:
     Span *prev = nullptr;
-    for (Span *sd = disk[i]; sd;) {
+    for (Span *sd = spans[i]; sd;) {
       Span *next = sd->link.next;
       if (next &&
           ((strcmp(sd->pathname, next->pathname) < 0) || 
(!strcmp(sd->pathname, next->pathname) && sd->offset > next->offset))) {
         if (!prev) {
-          disk[i]         = next;
+          spans[i]        = next;
           sd->link.next   = next->link.next;
           next->link.next = sd;
         } else {
@@ -136,8 +135,8 @@ Store::sort()
 
   // merge adjacent spans
 
-  for (unsigned i = 0; i < n_disks; i++) {
-    for (Span *sd = disk[i]; sd;) {
+  for (unsigned i = 0; i < n_spans; i++) {
+    for (Span *sd = spans[i]; sd;) {
       Span *next = sd->link.next;
       if (next && !strcmp(sd->pathname, next->pathname)) {
         if (!sd->file_pathname) {
@@ -197,14 +196,14 @@ Span::volume_number_set(int n)
 void
 Store::delete_all()
 {
-  for (unsigned i = 0; i < n_disks; i++) {
-    if (disk[i]) {
-      delete disk[i];
+  for (unsigned i = 0; i < n_spans; i++) {
+    if (spans[i]) {
+      delete spans[i];
     }
   }
-  n_disks = 0;
-  ats_free(disk);
-  disk = nullptr;
+  n_spans = 0;
+  ats_free(spans);
+  spans = nullptr;
 }
 
 Store::~Store()
@@ -261,7 +260,7 @@ Store::read_config()
 
     // parse
     Dbg(dbg_ctl_cache_init, "Store::read_config: \"%s\"", path);
-    ++n_disks_in_config;
+    ++n_spans_in_config;
 
     int64_t size   = -1;
     int volume_num = -1;
@@ -334,7 +333,7 @@ Store::read_config()
   while (cur) {
     Span *next     = cur->link.next;
     cur->link.next = nullptr;
-    disk[i++]      = cur;
+    spans[i++]     = cur;
     cur            = next;
   }
   sd = nullptr; // these are all used.
@@ -348,8 +347,8 @@ Store::read_config()
 int
 Store::write_config_data(int fd) const
 {
-  for (unsigned i = 0; i < n_disks; i++) {
-    for (Span *sd = disk[i]; sd; sd = sd->link.next) {
+  for (unsigned i = 0; i < n_spans; i++) {
+    for (Span *sd = spans[i]; sd; sd = sd->link.next) {
       char buf[PATH_NAME_MAX + 64];
       snprintf(buf, sizeof(buf), "%s %" PRId64 "\n", sd->pathname.get(), 
sd->blocks * static_cast<int64_t>(STORE_BLOCK_SIZE));
       if (ink_file_fd_writestring(fd, buf) == -1) {
@@ -486,7 +485,6 @@ Span::init(const char *path, int64_t size)
   }
 
   // A directory span means we will end up with a file, otherwise, we get what 
we asked for.
-  this->set_mmapable(ink_file_is_mmappable(S_ISDIR(sbuf.st_mode) ? 
static_cast<mode_t>(S_IFREG) : sbuf.st_mode));
   this->pathname = ats_strdup(path);
 
   Dbg(dbg_ctl_cache_init, "initialized span '%s'", this->pathname.get());
@@ -499,13 +497,3 @@ Span::init(const char *path, int64_t size)
 fail:
   return Span::errorstr(serr);
 }
-
-Span *
-Span::dup()
-{
-  Span *ds = new Span(*this);
-  if (this->link.next) {
-    ds->link.next = this->link.next->dup();
-  }
-  return ds;
-}

Reply via email to