Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DiskBasedCache uses different inputs for file size #57

Closed
jpd236 opened this issue Jul 18, 2017 · 1 comment
Closed

DiskBasedCache uses different inputs for file size #57

jpd236 opened this issue Jul 18, 2017 · 1 comment
Milestone
1.2.0

Comments

@jpd236
Copy link
Collaborator

jpd236 commented Jul 18, 2017

Forking from #52 (comment):

Some parts of DiskBasedCache use the file size as the size of the cache entry, which includes the header overhead. Other parts use the data size which doesn't include the header overhead.

We should make this sane. Some constraints:

  • I'd generally prefer we use an accurate size on disk since we advertise the cache as using at most that size, but not including the overhead means we might exceed it.
  • It's currently hard to calculate the size prior to writing the entry because we don't know how big the CacheHeader will be. I think it would be technically feasible to calculate this deterministically though given that serialization format is fixed (or at least to estimate it...)
@jpd236 jpd236 mentioned this issue Jul 18, 2017
Merged
@jpd236 jpd236 modified the milestones: 1.0.1, 1.1.0 Sep 21, 2017
@jpd236 jpd236 modified the milestones: 1.2.0, 1.1.1 Oct 12, 2017
@joebowbeer joebowbeer mentioned this issue Feb 23, 2018
Closed
jpd236 added a commit to jpd236/volley that referenced this issue May 31, 2018
@jpd236
Enforce disk size limit more strictly in DiskBasedCache.
1476fac 
-Previously, when new entries were written to the cache with put(),
the size accounted for in those entries did not include cache overhead
when actually storing them on disk. That overhead is now included.

-We now prune excess entries after writing new entries to disk instead
of before (so that we know the size-on-disk). This prevents the cache
from exceeding the size limit due to one large entry (until the next
put). The cache will now exceed the maximum size when writing an entry
which pushes it over the limit; however, this excess will be cleared
before put() returns rather than some indefinite time in the future.

Fixes google#57
jpd236 added a commit to jpd236/volley that referenced this issue May 31, 2018
@jpd236
Enforce disk size limit more strictly in DiskBasedCache.
0e6b372 
-Previously, when new entries were written to the cache with put(),
the size accounted for in those entries did not include cache overhead
when actually storing them on disk. That overhead is now included.

-We now prune excess entries after writing new entries to disk instead
of before (so that we know the size-on-disk). This prevents the cache
from exceeding the size limit due to one large entry (until the next
put). The cache will now exceed the maximum size when writing an entry
which pushes it over the limit; however, this excess will be cleared
before put() returns rather than some indefinite time in the future.

Fixes google#57
@jpd236 jpd236 mentioned this issue May 31, 2018
Merged
@jpd236
Copy link
Collaborator Author

jpd236 commented May 31, 2018

@joebowbeer - I can't seem to request your review through GitHub, but if you'd like to take a look at #196, this should make things consistent around the size on disk. Rather than calculate the header size (outside of the unit tests), I instead just serialize the new entry to disk, take its size, and then prune the cache to fit afterwards.

@jpd236 jpd236 modified the milestones: 1.1.1, 1.2.0 May 31, 2018
jpd236 added a commit that referenced this issue Aug 17, 2018
@jpd236
Enforce disk size limit more strictly in DiskBasedCache.
6b7b3a2 
-Previously, when new entries were written to the cache with put(),
the size accounted for in those entries did not include cache overhead
when actually storing them on disk. That overhead is now included.

-We now prune excess entries after writing new entries to disk instead
of before (so that we know the size-on-disk). This prevents the cache
from exceeding the size limit due to one large entry (until the next
put). The cache will now exceed the maximum size when writing an entry
which pushes it over the limit; however, this excess will be cleared
before put() returns rather than some indefinite time in the future.

Fixes #57
This was referenced Mar 9, 2021
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

No branches or pull requests
1 participant
@jpd236