Skip to content

Commit be9b80c

Browse files
authored
Prevent packed weight cache file from growing on reload (pytorch#20833)
Summary: On each NGTTS model reload, the cache file grew by ~24KB and the timestamp updated, even though all named entries were cache hits. Three independent causes: 1. Unnamed constants (not registered in `NamedDataMap`) always miss `look_up` and go through `reserve_space` file-backed path, extending the file with data that can never be found by name on subsequent loads. 2. After `load_packed_cache`, `mmap_regions_synced_` was 0 while `mmap_regions_` was 1, causing `finalize_for_runtime` to msync the read-only loaded region on every reload — updating the file mtime. 3. XNNPACK internally re-packs certain weights (e.g., for different runtime contexts) by calling `reserve_space` without going through `look_up` first. On a warm cache this extends the file with data already present in the loaded cache. Fix: - `last_lookup_unnamed_`: set in `look_up` when the kernel pointer is not in the name map. `reserve_space` checks this flag and routes to heap. - `mmap_regions_synced_`: initialized in `load_packed_cache` to match `mmap_regions_.size()`, preventing spurious msync on loaded regions. - `loaded_from_disk_`: set after `load_packed_cache` succeeds. `reserve_space` routes to heap when the cache is warm — all named entries are already in the file, so any new `reserve_space` call is a re-pack that does not need to persist. Differential Revision: D111354375
1 parent 3d9936e commit be9b80c

2 files changed

Lines changed: 20 additions & 0 deletions

File tree

backends/xnnpack/runtime/XNNWeightsCache.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,10 @@ size_t XNNWeightsCache::look_up(
340340
const void* unpacked_bias_ptr = cache_key->bias;
341341
auto entry = context->unpacked_data_to_name_.find(unpacked_weights_ptr);
342342
if (entry == context->unpacked_data_to_name_.end()) {
343+
context->last_lookup_unnamed_ = true;
343344
return SIZE_MAX;
344345
}
346+
context->last_lookup_unnamed_ = false;
345347
std::string weight_bias_name = entry->second;
346348

347349
if (unpacked_bias_ptr != nullptr) {
@@ -375,6 +377,9 @@ size_t XNNWeightsCache::look_up(
375377

376378
void* XNNWeightsCache::reserve_space(XNNWeightsCache* context, size_t n) {
377379
#ifndef _WIN32
380+
if (context->last_lookup_unnamed_ || context->loaded_from_disk_) {
381+
return context->reserve_space_heap(n);
382+
}
378383
if (context->packed_file_fd_ >= 0) {
379384
size_t page_size = sysconf(_SC_PAGESIZE);
380385
size_t file_offset =
@@ -753,6 +758,8 @@ bool XNNWeightsCache::load_packed_cache() {
753758
// a no-op until new packs arrive. Initialize watermark so
754759
// save_packed_index short-circuits.
755760
mmap_regions_at_last_save_ = mmap_regions_.size();
761+
mmap_regions_synced_ = mmap_regions_.size();
762+
loaded_from_disk_ = true;
756763
return true;
757764
#else
758765
return false;

backends/xnnpack/runtime/XNNWeightsCache.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,19 @@ class XNNWeightsCache {
223223
// in mmap_regions_, so delete_packed_data() can munmap when ref_count==0.
224224
std::unordered_map<void*, size_t> file_ptr_to_region_index_;
225225

226+
// Set by look_up when the kernel pointer is not in unpacked_data_to_name_
227+
// (i.e., the constant is unnamed / embedded in the delegate blob). Checked
228+
// by reserve_space to route unnamed constants to heap instead of the
229+
// file-backed mmap — unnamed entries can never be found by name on reload,
230+
// so persisting them to disk wastes space and triggers spurious saves.
231+
bool last_lookup_unnamed_{false};
232+
233+
// True after load_packed_cache succeeds. When set, reserve_space routes
234+
// to heap — all named entries are already in the file, so any new
235+
// reserve_space is a re-pack (e.g., XNNPACK internal re-pack for a
236+
// different runtime context) that doesn't need to persist.
237+
bool loaded_from_disk_{false};
238+
226239
// See mutex() for the locking contract — caller-owned, no internal
227240
// use within this class.
228241
std::mutex instance_mutex_;

0 commit comments

Comments
 (0)