Persist seed-mismatch re-packs so the packed cache converges after an XNNPACK upgrade (#22183)

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint

[doggeral]

Summary:

After an XNNPACK upgrade the per-ukernel seed changes, so every entry loaded
from the packed weight cache misses look_up (seed mismatch) and XNNPACK
re-packs it. reserve_space routes all re-packs to heap whenever
loaded_from_disk_ is set, and save_packed_index only persists when new mmap
regions were added — so the refreshed packing is never written back. Every
launch then reloads the same stale file, re-packs all weights into anonymous
(dirty) heap, and, when backgrounded, can be OOM/jetsam-killed. The cache
never converges.

Fix: track a per-look_uplast_lookup_seed_mismatch_ flag, set when a named
entry is present but its cached seed differs from the current ukernel's seed.
reserve_space keeps unnamed constants and incidental warm re-packs on heap as
before, but routes a seed-mismatch re-pack to the file-backed path. This lets
save_packed_index persist the refreshed seed, so the next launch hits and the
cache converges after a single rebuild, with the re-pack landing in
reclaimable file-backed pages instead of anonymous dirty memory.

Note: the old packed bytes of re-packed entries remain as orphans, so the
file grows ~once per XNNPACK upgrade (see the file_bytes logging in
save_packed_index); orphan GC/compaction is a follow-up.

---

V2 — addressing review comments

1. "For this 'miss' — can you ever get here with loaded_from_disk_ == true?"

Yes, and that is exactly why the loaded_from_disk_ clause has to stay. Two
ways in:

• A named entry that is simply absent from the loaded index (a weight the
  cached PTE never had) misses look_up with both flags false.
• More importantly, per D111354375 cause Rename _pt2e to pt2e #3, XNNPACK re-packs some weights for
  a different runtime context by calling reserve_space with no preceding
  look_up at all.

That second case exposed a bug in V1: with no look_up to reset it,
last_lookup_seed_mismatch_ stayed true from an earlier lookup and would
route such a re-pack to the file, re-growing the cache on every load — the very
regression D111354375 fixed. reserve_space now consumes the flag
(reads it, then clears it), so only the re-pack that directly follows a
seed-mismatch look_up takes the file-backed path.

2. Writing the tests surfaced a pre-existing bug that made V1 a no-op

load_packed_cache takes flock(LOCK_SH), mmaps the file MAP_SHARED, then
close(fd). The live mapping keeps the open file description — and therefore
the flock — alive, so close() never drops the lock. The open_locked(..., O_RDWR) that follows in initialize_for_runtime then always fails with
EWOULDBLOCK, leaving packed_file_fd_ == -1. Confirmed:

LOCK_SH -> mmap -> close -> LOCK_EX => EWOULDBLOCK
LOCK_SH -> close -> LOCK_EX => OK

With packed_file_fd_ == -1, reserve_space's file-backed branch is
unreachable and save_packed_index early-returns, so every write path after
a warm load was dead — including the seed-mismatch re-pack this diff adds.
The flock(...) failed (errno=11) line is present in the logs of every
existing cache test, right after "Loaded packed weight cache"; nothing asserted
on it.

Fixed with an explicit flock(fd, LOCK_UN) before the close(fd), which
preserves exactly the lock scope the original close() placement intended.

3. Tests

Two new tests in test_xnn_weights_cache.cpp:

• SeedMismatch_RepackIsPersistedToFile — tamper the persisted seed to
  simulate the upgrade, then assert the next launch writes the refreshed seed
  back and appends the re-packed bytes to the file (i.e. it did not land on
  heap), with unchanged inference output.
• SeedMismatch_CacheConvergesAfterOneRebuild — assert exactly one launch pays
  for the rebuild and the three launches after it are pure hits: no file
  growth, seed stable, output stable.

Both fail on V1's routing logic and pass with the fix. Also factored the
duplicated footer/seed-field walking in the existing tests into
read_first_entry_seed / write_first_entry_seed helpers (the old inline
corruption write was host-endian; the helper is little-endian like the format).

Reviewed By: JakeStevens

Differential Revision: D117550758

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22183

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@doggeral has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117550758.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

please fix lint