feat: New WithMaxCost option for custom cache capacity management strategies

[dadrus]

closes #135

This PR introduces a new WithMaxCost option, enabling the restriction of the cache's capacity based on factors beyond just the number of items it stores. This opens up the possibility for implementing custom strategies. For example, one such strategy could involve setting a memory limit for the cache, as demonstrated in the following snippet.

import (
    "github.com/jellydator/ttlcache"
    "github.com/DmitriyVTitov/size"
)

func main() {
    cache := ttlcache.New[string, string](
        ttlcache.WithMaxCost[string, string](5120, func(item *ttlcache.Item[string, string]) uint64 {
            return uint64(size.Of(item))
        }), 
    )

    cache.Set("first", "value1", ttlcache.DefaultTTL)
}

With this configuration in place, if an existing item is updated with a new value or a new item is added, causing memory exhaustion (5KiB here), older items will be evicted. In such cases, the eviction reason will be EvictionReasonMaxCostExceeded.

[dadrus]

@davseby , @swithek: As you can see in the PR description, tests is an open topic. I actually started implementing those locally, but run in some limitations related to the structure of the available tests (they share too much state in https://github.com/dadrus/ttlcache/blob/6e92deb3b7e755ab332e11670c82b0290cab626f/cache_test.go#L261-L326). Therefore, I would like to discuss a couple of topics with you first:

• Naming of the option and properties I introduced in the PR.
• Making the tests more future proof - I think it would be easier to maintain the tests if each test case could implement its own setup and assertions, similar to the implementation, you can find here: https://github.com/dadrus/heimdall/blob/main/internal/cache/memory/cache_test.go
• What should we do, if calculation of memory size for a given item fails

[swithek]

Hi, thanks for the PR! And sorry for the delayed review.

My main suggestion would be to change the name/concept of this feature from "size" to "weight" or "cost". This way this new functionality could be a bit more versatile: some users might want to track memory usage, others -- some other piece of data that determines how their resources are/should be used.

For this to work properly, we would need another functional option: WithCostFunc(fn func(item)) (or something similar). The provided function would be used to determine the cost/weight (or size) of every item. This way the users would be able to use external libraries without introducing them into our codebase as well.

[swithek]

As for the tests: if the improvements are trivial, feel free to implement them, if not, then just ensure that the new functionality is properly covered.

[dadrus]

@swithek: You're very welcome :) and excuse me for the belated answer and the updates as well.

I’ve updated the implementation based on your suggestion, with a couple of deviations, which are close to the suggestion in #135 (comment):

The WithTotalCost function signature and the cost calculation function differ slightly:

• The cost calculation function now accepts two parameters—the key and the item—since both influence the total memory usage in my case.
• WithTotalCost takes the cost calculation function as its second parameter. Adding a separate option for setting only the calculation function would leave the configuration incomplete.

Additionally, there are further other points worth discussing:

• With this new functionality, WithCapacity could potentially be replaced by the new option. For instance, it could be implemented internally as:

  func WithCapacity[K comparable, V any](c uint64) Option[K, V] {
      return WithTotalCost(c, func(key K, item V) uint64 { return 1 })
  }

  Alternatively, it might even be removed entirely to simplify the codebase, though this could be a future consideration.

• As you can see, I’ve added additional test cases to cover the new functionality. However, I was unable to redesign the entire test function, as it’s unclear what the specific expectations are for each of the existing test cases. If you could provide comments describing the intended expectations for each case, I’d be glad to work on a complete redesign of the test function to improve clarity and maintainability.

• Personally, I find the name WithMaxCost or WithMaxWeight (as suggested by @davseby in Make the cache size configurable #135 (comment)) more concise and descriptive for the new option, though that’s subjective.

While I don’t think the first two points need to be addressed in this PR, I’d appreciate your feedback on all of them.

[swithek]

@dadrus these are some good additions 👍 Just a few more changes and I think it should be ready to be merged.

[swithek]

With this new functionality, WithCapacity could potentially be replaced by the new option. For instance, it could be implemented internally as: [...]
Alternatively, it might even be removed entirely to simplify the codebase, though this could be a future consideration.

If the new WithMaxCost is used the same way WithCapacity is, then yes, WithCapacity becomes redundant. However, what if someone wanted to have a capacity check and an additional cost (e.g., size, as in your use case) check? The overlap between these two features isn't complete, so removing one of them, or even integrating one of them into another isn't a very good option.

As you can see, I’ve added additional test cases to cover the new functionality. However, I was unable to redesign the entire test function, as it’s unclear what the specific expectations are for each of the existing test cases. If you could provide comments describing the intended expectations for each case, I’d be glad to work on a complete redesign of the test function to improve clarity and maintainability.

That's perfectly fine, the redesign of the test suite isn't a critical task for now (it's not even a task at the moment), so no need spend too much time on it, especially in this PR. As for expectations of each test case, I don't think I can add much at the moment either; each case has a decent description, so that should give a big enough clue.

[dadrus]

@swithek: Please excuse me for the longer silence. The new updates implement the approach discussed in #152 (comment) and ff. Please review

[swithek]

Looks good, left a few comments.

I think for now it's best to make the new item func and its options private (marked by comments). I'll come back to this myself once this PR is merged. There are a few other features in the queue that might benefit from this, so I'd like to take everything into consideration and revise the signatures if needed before finalising these types/funcs. Besides, this PR is large enough already.

[dadrus]

@swithek: All your requests have been addressed. As you may have noticed, I’ve also updated the title and description of this PR. Please have another look.

[swithek]

I think this is ready to be merged. @dadrus thank you for your patience and all the revisions you made. This is probably the biggest PR in recent months, so congrats on successfully tackling this challenge and finishing it!

I'll squash-merge this to the v3 branch, so it will be possible to download it from there directly. However, the next major version (v3.4) will be released in a few weeks, since there's a plan to include more features in it.

[dadrus]

I had to delete some of my "review" comments that you responded to in #152 (comment) because gh wouldn't let me resolve them otherwise.

I don’t have any additional topics to bring up, but I want to thank you for your patience and the many reviews you’ve provided to help shape this PR into what it is now.

If you don’t have anything else either, let’s go ahead and squash it :)