Skip to content

Dedicated method with state as an input#122

Closed
fsdvh wants to merge 9 commits into
arthurprs:masterfrom
fsdvh:state-method-branch
Closed

Dedicated method with state as an input#122
fsdvh wants to merge 9 commits into
arthurprs:masterfrom
fsdvh:state-method-branch

Conversation

@fsdvh

@fsdvh fsdvh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds insert_with_state and try_insert_with_state — variants of the existing _with_lifecycle methods that accept a caller-owned &mut L::RequestState instead of creating one internally. The existing _with_lifecycle methods are refactored to delegate to these, eliminating duplicated shard-lookup and insert logic.

Motivation

The _with_lifecycle methods give callers control over when evicted items are dropped (outside the shard lock). However, they each create their own lifecycle request, so callers who need to batch multiple insertions under a single begin_request/end_request pair had no supported way to do so.

The new _with_state methods fill this gap:

let mut lcs = cache.lifecycle.begin_request();
cache.insert_with_state(key1, val1, &mut lcs);
cache.insert_with_state(key2, val2, &mut lcs);
cache.lifecycle.end_request(lcs); // all evictions dropped here, once

Changes

  • New: insert_with_state(&self, key, value, lcs: &mut L::RequestState) — blocking insert into an existing lifecycle request.
  • New: try_insert_with_state(&self, key, value, lcs: &mut L::RequestState) -> Result<(), (Key, Val)> — non-blocking variant; returns inputs on lock contention.
  • Refactor: insert_with_lifecycle and try_insert_with_lifecycle now delegate to the new methods.
  • Docs: Improved doc comments for with_weighter, replace_with_lifecycle, insert_with_lifecycle, get_or_insert_with, and get_or_insert_async.

fsdvh and others added 8 commits April 13, 2026 10:39
Add non-blocking API for sync cache
Expose shard_index method
* Add non-blocking methods

* Do not leak lcs

* Dedicated result

* More

* Simplify

* Cleanup + some docs

* Docs

* Use a feature flag

* Posion lock

* Remove feature and more tests

* Update src/sync.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Simplify signature

* Simplify

* clippy

* More docs

* Update README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/sync.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* AI suggestions

* Update src/sync.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Dedicated method with state as an input

* Shared state methods

* More docs
arthurprs added a commit that referenced this pull request Jun 24, 2026
BREAKING (0.7.0). Collapses the growing per-operation variant cross-product.

- `Lifecycle::RequestState` now requires `Default`; `begin_request` and
  `end_request` are removed. State is built via `Default` and finalized by
  its own `Drop` (which releases evicted items outside the shard lock).
- The `*_with_lifecycle` methods now take `&mut L::RequestState` (returning
  `()` / `Result<(), _>`) instead of returning the state, so one state can be
  threaded through several ops to batch eviction work or inspect evicted items.
  This supersedes the `_with_state` proposal in #122.
- Drops the now-vestigial `&L` plumbing from the placeholder machinery and the
  unused `Cache::lifecycle` field; shards keep their own clones for the hooks.
@arthurprs

Copy link
Copy Markdown
Owner

This is a good point and highlights that the API is limited. I'm leaning towards changing the lifecycles APIs to take &mut as you have here (See #124). That's a breaking change but justified.

@fsdvh

fsdvh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

This is a good point and highlights that the API is limited. I'm leaning towards changing the lifecycles APIs to take &mut as you have here (See #124). That's a breaking change but justified.

I really like your proposed changes. Happy to close my PR in favour of yours!

@fsdvh

fsdvh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #124

@fsdvh fsdvh closed this Jun 24, 2026
arthurprs added a commit that referenced this pull request Jun 25, 2026
…124)

* Reshape lifecycle API: Default-built RequestState threaded by &mut

BREAKING (0.7.0). Collapses the growing per-operation variant cross-product.

- `Lifecycle::RequestState` now requires `Default`; `begin_request` and
  `end_request` are removed. State is built via `Default` and finalized by
  its own `Drop` (which releases evicted items outside the shard lock).
- The `*_with_lifecycle` methods now take `&mut L::RequestState` (returning
  `()` / `Result<(), _>`) instead of returning the state, so one state can be
  threaded through several ops to batch eviction work or inspect evicted items.
  This supersedes the `_with_state` proposal in #122.
- Drops the now-vestigial `&L` plumbing from the placeholder machinery and the
  unused `Cache::lifecycle` field; shards keep their own clones for the hooks.

* Drop RequestState outside the lock in set_capacity

set_capacity created and dropped its own RequestState inside the shard
method, so when called via Cache::set_capacity (shard.write().set_capacity)
the evicted items were released while the write lock was still held —
contradicting the documented Drop-after-lock contract. Take &mut lcs from
the caller instead, so it drops after the lock guard.

Also use Default::default() consistently instead of L::RequestState::default().

* Document lcs/lock drop ordering in set_capacity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants