New issue
Advanced search Search tips

Issue 761454 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

simple cache size does not increase after WriteData

Project Member Reported by cmumford@chromium.org, Sep 1 2017

Issue description

If data is successfully written to an entry (via WriteData) the entry size should increase - it does not.

This is a result of a recent change https://codereview.chromium.org/2957133002.

Here is a modification to an existing unit test that demonstrates this problem:

void DiskCacheBackendTest::BackendCalculateSizeOfAllEntries() {
  ...
  const std::string key = "A Key";
  {
    disk_cache::Entry* entry;
    EXPECT_NE(net::OK, OpenEntry(key, &entry));
    ASSERT_THAT(CreateEntry(key, &entry), IsOk());
    ASSERT_TRUE(nullptr != entry);
    entry->Close();
  }
  int size_after_write = CalculateSizeOfAllEntries();
  EXPECT_GT(size_after_write, 0);
  // Now write some data at a given index.
  {
    disk_cache::Entry* entry;
    ASSERT_THAT(OpenEntry(key, &entry), IsOk());
    ASSERT_TRUE(nullptr != entry);
    const std::string data = "SideDataSample";
    scoped_refptr<net::IOBuffer> buffer(new net::StringIOBuffer(data));
    const int buf_length = data.length();
    const int index = 0;
    EXPECT_EQ(buf_length, WriteData(entry, index, 0 /* offset */, buffer.get(),
                                    buf_length, false));
    entry->Close();
    EXPECT_GT(CalculateSizeOfAllEntries(), size_after_write)
        << "Writing " << buf_length << " bytes of data to index " << index
        << " did not increase the backend size.";
  }
}
 
Blocking: 617963
Owner: morlovich@chromium.org
Status: Assigned (was: Untriaged)
Hmm, don't have the bits to see the bug this blocks.

So this might be "expected" with respect to my mental model, except I suspect that one assumption I made may be bogus for ServiceWorker. Basically my argument to myself was that it's OK to round up file sizes a little bit to save memory (or rather to do some performance work without wasting memory), since this API is referring to implementation-specific disk footprint --- not the user's data sizes. It would be perfectly reasonable for the implementation to pad files some for alignment, so it felt reasonable to basically act as-if we did that w/o actually changing the on-disk format.

(I suspect it may break down for your needs since you may need to do something with disk directly?)


Only tests have a dependency on file sizes, not shipping code. I modified the tests to write much larger data to the cache (>= 1KB), so if the file size is quantized it shouldn't cause a failure.
Blocking: -617963
Unblocking 617963. I see the point being made in #c2, but it seems as though we've now introduced a distinction between physical and logical entry size, but have no API to reflect this. It may not be a big issue, but I suspect others maybe confused as I was by this.

Sign in to add a comment