[wifi] Use memcpy-based insertion sort for scan results

Replace copy-assignment with raw memcpy in the WiFi scan result
insertion sort. Copy assignment on WiFiScanResult calls
CompactString's destructor then placement-new for every shift,
which means delete[]/new[] per shift for heap-allocated SSIDs.

With 70+ networks visible (e.g., during captive portal transition
showing full scan results), this caused event loop blocking from
hundreds of heap allocations in a tight loop on an 80MHz ESP8266.

This optimization is safe because we're permuting elements within
the same array - each slot is overwritten exactly once, so no
ownership duplication occurs. CompactString stores either inline
data or a heap pointer, never a self-referential pointer (unlike
libstdc++ std::string SSO). This was made possible by PR#13472
which replaced std::string with CompactString.

Static asserts guard the memcpy safety assumptions at compile time.

Confirmed on real device: event loop blocking during captive portal
transition is eliminated and WiFi connection is slightly faster.
This commit is contained in:
J. Nick Koston
2026-02-12 12:21:52 -06:00
parent 1604b5d6e4
commit 58f8029264
2 changed files with 48 additions and 4 deletions

View File

@@ -1319,20 +1319,58 @@ void WiFiComponent::start_scanning() {
// Using insertion sort instead of std::stable_sort saves flash memory
// by avoiding template instantiations (std::rotate, std::stable_sort, lambdas)
// IMPORTANT: This sort is stable (preserves relative order of equal elements)
//
// Uses raw memcpy instead of copy assignment to avoid CompactString's
// destructor/constructor overhead (heap delete[]/new[] for long SSIDs).
// Copy assignment calls ~CompactString() then placement-new for every shift,
// which means delete[]/new[] per shift for heap-allocated SSIDs. With 70+
// networks (e.g., captive portal showing full scan results), this caused
// event loop blocking from hundreds of heap operations in a tight loop.
//
// This is safe because we're permuting elements within the same array —
// each slot is overwritten exactly once, so no ownership duplication occurs.
// All members of WiFiScanResult are either trivially copyable (bssid, channel,
// rssi, priority, flags) or CompactString, which stores either inline data or
// a heap pointer — never a self-referential pointer (unlike std::string's SSO
// on some implementations). This was not possible before PR#13472 replaced
// std::string with CompactString, since std::string's internal layout is
// implementation-defined and may use self-referential pointers.
template<typename VectorType> static void insertion_sort_scan_results(VectorType &results) {
// memcpy-based sort requires no self-referential pointers or virtual dispatch.
// These static_asserts guard the assumptions. If any fire, the memcpy sort
// must be reviewed for safety before updating the expected values.
//
// No vtable pointers (memcpy would corrupt vptr)
static_assert(!std::is_polymorphic<WiFiScanResult>::value, "WiFiScanResult must not have vtable");
static_assert(!std::is_polymorphic<CompactString>::value, "CompactString must not have vtable");
// Standard layout ensures predictable memory layout with no virtual bases
// and no mixed-access-specifier reordering
static_assert(std::is_standard_layout<WiFiScanResult>::value, "WiFiScanResult must be standard layout");
static_assert(std::is_standard_layout<CompactString>::value, "CompactString must be standard layout");
// Size checks catch added/removed fields that may need safety review
static_assert(sizeof(WiFiScanResult) == 32, "WiFiScanResult size changed - verify memcpy sort is still safe");
static_assert(sizeof(CompactString) == 20, "CompactString size changed - verify memcpy sort is still safe");
// Alignment must match for reinterpret_cast of key_buf to be valid
static_assert(alignof(WiFiScanResult) <= alignof(std::max_align_t), "WiFiScanResult alignment exceeds max_align_t");
const size_t size = results.size();
constexpr size_t elem_size = sizeof(WiFiScanResult);
// Suppress warnings for intentional memcpy on non-trivially-copyable type.
// Safety is guaranteed by the static_asserts above and the permutation invariant.
// NOLINTNEXTLINE(bugprone-undefined-memory-manipulation)
auto *memcpy_fn = &memcpy;
for (size_t i = 1; i < size; i++) {
// Make a copy to avoid issues with move semantics during comparison
WiFiScanResult key = results[i];
alignas(WiFiScanResult) uint8_t key_buf[elem_size];
memcpy_fn(key_buf, &results[i], elem_size);
const auto &key = *reinterpret_cast<const WiFiScanResult *>(key_buf);
int32_t j = i - 1;
// Move elements that are worse than key to the right
// For stability, we only move if key is strictly better than results[j]
while (j >= 0 && wifi_scan_result_is_better(key, results[j])) {
results[j + 1] = results[j];
memcpy_fn(&results[j + 1], &results[j], elem_size);
j--;
}
results[j + 1] = key;
memcpy_fn(&results[j + 1], key_buf, elem_size);
}
}

View File

@@ -219,6 +219,12 @@ class CompactString {
};
static_assert(sizeof(CompactString) == 20, "CompactString must be exactly 20 bytes");
// CompactString is safe to memcpy for permutation-based sorting (no ownership duplication).
// Unlike libstdc++ std::string which uses a self-referential pointer (_M_p -> _M_local_buf)
// in SSO mode, CompactString stores either inline data or an external heap pointer in
// storage_[] — never a pointer to itself. These asserts guard that property.
static_assert(std::is_standard_layout<CompactString>::value, "CompactString must be standard layout for memcpy safety");
static_assert(!std::is_polymorphic<CompactString>::value, "CompactString must not have vtable for memcpy safety");
class WiFiAP {
friend class WiFiComponent;