Optimize sector cache

cpp/controllers/controller_factory.h

@@ -56,5 +56,5 @@ class ControllerFactory
     shared_ptr<ScriptGenerator> script_generator;
 
     spdlog::level::level_enum log_level = spdlog::get_level();
-    string log_pattern = "[%Y-%m-%d %H:%M:%S.%e] [%^%l%$] %n %v";
+    string log_pattern = "%n [%^%l%$] %v";
 };

cpp/devices/disk.cpp

@@ -8,7 +8,7 @@
 // XM6i
 //   Copyright (C) 2010-2015 [email protected]
 //   Copyright (C) 2010 Y.Sugahara
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -577,7 +577,7 @@ tuple<bool, uint64_t, uint32_t> Disk::CheckAndGetStartAndCount(AccessMode mode)
     }
 
     // Do not process 0 blocks
-    return tuple(count || mode == SEEK6 || mode == SEEK10, start, count);
+    return {count || mode == SEEK6 || mode == SEEK10, start, count};
 }
 
 vector<PbStatistics> Disk::GetStatistics() const

cpp/devices/disk_cache.cpp

@@ -8,7 +8,7 @@
 // XM6i
 //   Copyright (C) 2010-2015 [email protected]
 //   Copyright (C) 2010 Y.Sugahara
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -46,7 +46,7 @@ shared_ptr<DiskTrack> DiskCache::GetTrack(uint32_t block)
     int track = block >> 8;
 
     // Get track data
-    return Assign(track);
+    return AssignTrack(track);
 }
 
 int DiskCache::ReadSectors(data_in_t buf, uint64_t sector, uint32_t count)
@@ -81,12 +81,9 @@ int DiskCache::WriteSectors(data_out_t buf, uint64_t sector, uint32_t count)
     return disktrk->WriteSector(buf, sector & 0xff);
 }
 
-// Track Assignment
-shared_ptr<DiskTrack> DiskCache::Assign(int track)
+shared_ptr<DiskTrack> DiskCache::AssignTrack(int track)
 {
-    assert(track >= 0);
-
-    // First, check if it is already assigned
+    // Check if it is already assigned
     for (CacheData &c : cache) {
         if (c.disktrk && c.disktrk->GetTrack() == track) {
             // Track match
@@ -95,54 +92,29 @@ shared_ptr<DiskTrack> DiskCache::Assign(int track)
         }
     }
 
-    // Next, check for empty
-    for (size_t i = 0; i < cache.size(); ++i) {
-        if (!cache[i].disktrk) {
-            // Try loading
-            if (Load(static_cast<int>(i), track, nullptr)) {
-                // Success loading
-                cache[i].serial = serial;
-                return cache[i].disktrk;
-            }
-
-            // Load failed
-            return nullptr;
+    // Check for an empty cache slot
+    for (CacheData &c : cache) {
+        if (!c.disktrk && Load(static_cast<int>(&c - &cache[0]), track, nullptr)) {
+            c.serial = serial;
+            return c.disktrk;
         }
     }
 
-    // Finally, find the youngest serial number and delete it
-
-    // Set index 0 as candidate c
-    uint32_t s = cache[0].serial;
-    size_t c = 0;
+    // Find the cache entry with the smallest serial number, i.e. the oldest entry and save this track
+    if (auto c = ranges::min_element(cache,
+        [](const CacheData &d1, const CacheData &d2) {return d1.serial < d2.serial;})
+        - cache.begin(); cache[c].disktrk->Save(sec_path, cache_miss_write_count)) {
+        // Delete this track
+        auto disktrk = std::move(cache[c].disktrk);
 
-    // Compare candidate with serial and update to smaller one
-    for (size_t i = 0; i < cache.size(); ++i) {
-        assert(cache[i].disktrk);
-
-        // Compare and update the existing serial
-        if (cache[i].serial < s) {
-            s = cache[i].serial;
-            c = i;
+        if (Load(static_cast<int>(c), track, disktrk)) {
+            // Successful loading
+            cache[c].serial = serial;
+            return cache[c].disktrk;
         }
     }
 
-    // Save this track
-    if (!cache[c].disktrk->Save(sec_path, cache_miss_write_count)) {
-        return nullptr;
-    }
-
-    // Delete this track
-    shared_ptr<DiskTrack> disktrk = cache[c].disktrk;
-    cache[c].disktrk.reset();
-
-    if (Load(static_cast<int>(c), track, disktrk)) {
-        // Successful loading
-        cache[c].serial = serial;
-        return cache[c].disktrk;
-    }
-
-    // Load failed
+    // Save or load failed
     return nullptr;
 }
 

cpp/devices/disk_cache.h

@@ -5,7 +5,7 @@
 // Copyright (C) 2001-2006 ＰＩ．([email protected])
 // Copyright (C) 2014-2020 GIMONS
 //
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -37,7 +37,7 @@ class DiskCache : public Cache
         uint32_t serial;
     };
 
-    shared_ptr<DiskTrack> Assign(int);
+    shared_ptr<DiskTrack> AssignTrack(int);
     shared_ptr<DiskTrack> GetTrack(uint32_t);
     bool Load(int index, int track, shared_ptr<DiskTrack>);
     void UpdateSerial();

cpp/devices/disk_track.cpp

@@ -5,7 +5,7 @@
 // Copyright (C) 2001-2006 ＰＩ．([email protected])
 // Copyright (C) 2014-2020 GIMONS
 //
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -16,7 +16,7 @@
 
 DiskTrack::~DiskTrack()
 {
-    free(buffer); // NOSONAR free() must be used here because of allocation with posix_memalign
+    free(buffer); // NOSONAR free() must be used here due to posix_memalign
 }
 
 void DiskTrack::Init(int track, int size, int sectors)
@@ -41,27 +41,18 @@ bool DiskTrack::Load(const string &path, uint64_t &cache_miss_read_count)
 
     ++cache_miss_read_count;
 
-    // Calculate offset (previous tracks are considered to hold 256 sectors)
-    off_t offset = (off_t)track_number << 8;
-    offset <<= shift_count;
-
     const int size = sector_count << shift_count;
 
-    if (!buffer && !posix_memalign((void**)&buffer, 512, ((size + 511) / 512) * 512)) {
-        buffer_size = size;
-    }
-
-    // Reallocate if the buffer length is different
-    if (buffer && buffer_size != static_cast<uint32_t>(size)) {
-        free(buffer); // NOSONAR free() must be used here because of allocation with posix_memalign
+    // Allocate or reallocate the buffer
+    if (!buffer || buffer_size != static_cast<uint32_t>(size)) {
+        free(buffer); // NOSONAR free() must be used here due to posix_memalign
         buffer = nullptr;
-        if (!posix_memalign((void**)&buffer, 512, ((size + 511) / 512) * 512)) {
-            buffer_size = size;
+
+        if (posix_memalign((void**)&buffer, 512, (size + 511) & ~511)) {
+            return false;
         }
-    }
 
-    if (!buffer) {
-        return false;
+        buffer_size = size;
     }
 
     modified_flags.resize(sector_count);
@@ -74,6 +65,10 @@ bool DiskTrack::Load(const string &path, uint64_t &cache_miss_read_count)
         return false;
     }
 
+    // Calculate offset (previous tracks are considered to hold 256 sectors)
+    off_t offset = static_cast<off_t>(track_number) << 8;
+    offset <<= shift_count;
+
     in.seekg(offset);
     in.read((char*)buffer, size);
     return in.good();
@@ -102,18 +97,16 @@ bool DiskTrack::Save(const string &path, uint64_t &cache_miss_write_count)
     }
 
     // Write consecutive sectors
-    for (int i = 0; i < sector_count;) {
+    int i = 0;
+    while (i < sector_count) {
         if (modified_flags[i]) {
             int total = 0;
 
             // Determine consecutive sector range
-            int j;
-            for (j = i; j < sector_count; ++j) {
-                if (!modified_flags[j]) {
-                    break;
-                }
-
+            int j = i;
+            while (j < sector_count && modified_flags[j]) {
                 total += size;
+                ++j;
             }
 
             out.seekp(offset + (i << shift_count));

cpp/devices/page_handler.cpp

@@ -2,7 +2,7 @@
 //
 // SCSI2Pi, SCSI device emulator and SCSI tools for the Raspberry Pi
 //
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -87,8 +87,7 @@ int PageHandler::AddModePages(cdb_t cdb, data_in_t buf, int offset, int length,
 
     if (pages.contains(0)) {
         // Page data only (there is no standardized size field for page 0)
-        const auto &page_data = pages[0];
-        result.insert(result.end(), page_data.cbegin(), page_data.cend());
+        result.insert(result.end(), pages[0].cbegin(), pages[0].cend());
     }
 
     if (static_cast<int>(result.size()) > max_size) {
@@ -106,6 +105,8 @@ map<int, vector<byte>> PageHandler::GetCustomModePages(const string &vendor, con
 {
     map<int, vector<byte>> pages;
 
+    const string identifier = vendor + COMPONENT_SEPARATOR + product;
+
     for (const auto& [key, value] : PropertyHandler::Instance().GetProperties()) {
         const auto &key_components = Split(key, '.', 3);
 
@@ -119,8 +120,7 @@ map<int, vector<byte>> PageHandler::GetCustomModePages(const string &vendor, con
             continue;
         }
 
-        if (const string identifier = vendor + COMPONENT_SEPARATOR + product; !identifier.starts_with(
-            key_components[2])) {
+        if (!identifier.starts_with(key_components[2])) {
             continue;
         }
 

cpp/devices/tape.cpp

@@ -684,10 +684,9 @@ SimhMetaData Tape::FindNextObject(ObjectType type_to_find, int32_t requested_cou
             if (type_to_find == ObjectType::END_OF_DATA) {
                 return meta_data;
             }
-            else {
-                // End-of-data while spacing over something else
-                RaiseEndOfData(type_to_find, requested_count);
-            }
+
+            // End-of-data while spacing over something else
+            RaiseEndOfData(type_to_find, requested_count);
         }
         else if (type_found == ObjectType::FILEMARK && type_to_find == ObjectType::BLOCK) {
             // Terminate while spacing over blocks and a filemark is found
@@ -849,22 +848,18 @@ void Tape::Erase()
     file.seekp(tape_position);
 
     // Erase in chunks, using SIMH gaps as a pattern (little endian)
-    vector<uint8_t> buf;
-    buf.reserve(1024 * sizeof(SimhMarker::ERASE_GAP));
+    vector<uint8_t> buf(1024 * sizeof(SimhMarker::ERASE_GAP));
     const auto gap = static_cast<uint32_t>(SimhMarker::ERASE_GAP);
-    for (int i = 0; i < 1024; ++i) {
-        buf.push_back(gap & 0xff);
-        buf.push_back((gap >> 8) & 0xff);
-        buf.push_back((gap >> 16) & 0xff);
-        buf.push_back((gap >> 24) & 0xff);
+    for (size_t i = 0; i < buf.size(); i += 4) {
+        buf[i] = gap & 0xff;
+        buf[i + 1] = (gap >> 8) & 0xff;
+        buf[i + 2] = (gap >> 16) & 0xff;
+        buf[i + 3] = (gap >> 24) & 0xff;
     }
 
     uint64_t remaining = file_size - tape_position;
     while (remaining >= 4) {
-        uint64_t chunk = remaining;
-        if (chunk > buf.size()) {
-            chunk = buf.size();
-        }
+        const uint64_t chunk = min(remaining, buf.size());
 
         file.write((const char*)buf.data(), chunk);
         CheckForWriteError();
@@ -975,7 +970,7 @@ uint32_t Tape::CheckBlockLength()
 
             GetController()->SetStatus(StatusCode::CHECK_CONDITION);
 
-            return record_length < byte_count ? record_length : byte_count;
+            return min(record_length, byte_count);
         }
 
         // Report CHECK CONDITION if SILI is not set and the actual length does not match the requested length.
@@ -988,7 +983,7 @@ uint32_t Tape::CheckBlockLength()
 
             GetController()->SetStatus(StatusCode::CHECK_CONDITION);
 
-            return record_length < byte_count ? record_length : byte_count;
+            return min(record_length, byte_count);
         }
     }
 

cpp/s2p/s2p_core.cpp

@@ -38,7 +38,7 @@ bool S2p::InitBus(bool in_process, bool log_signals)
         return false;
     }
 
-    s2p_logger = CreateLogger("[" + APP_NAME + "]");
+    s2p_logger = CreateLogger(APP_NAME);
 
     executor = make_unique<CommandExecutor>(*bus, controller_factory, *s2p_logger);
 
@@ -408,8 +408,6 @@ void S2p::AttachInitialDevices(PbCommand &command)
         }
 #endif
     }
-
-    dispatcher->SetLogLevel(log_level);
 }
 
 bool S2p::CheckActive(const property_map &properties, const string &id_and_lun)

cpp/shared/s2p_formatter.cpp

@@ -21,10 +21,7 @@ string S2pFormatter::FormatBytes(span<const uint8_t> bytes, size_t count, bool h
 
     string str;
 
-    size_t limit = format_limit;
-    if (limit > count) {
-        limit = count;
-    }
+    size_t limit = min(static_cast<size_t>(format_limit), count);
 
     size_t offset = 0;
     while (offset < limit) {
@@ -33,11 +30,11 @@ string S2pFormatter::FormatBytes(span<const uint8_t> bytes, size_t count, bool h
         string output_ascii;
 
         if (!hex_only && !(offset % 16)) {
-            output_offset += fmt::format("{:08x}  ", offset);
+            output_offset = fmt::format("{:08x}  ", offset);
         }
 
-        size_t index = -1;
-        while (++index < 16 && offset < limit) {
+        size_t index = 0;
+        while (index < 16 && offset < limit) {
             if (index) {
                 output_hex += ":";
             }
@@ -46,6 +43,7 @@ string S2pFormatter::FormatBytes(span<const uint8_t> bytes, size_t count, bool h
             output_ascii += isprint(bytes[offset]) ? string(1, static_cast<char>(bytes[offset])) : ".";
 
             ++offset;
+            ++index;
         }
 
         str += output_offset;