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; -}