isolateserver: DiskCache format v2

isolateserver: DiskCache format v2

- change DiskCache._lru from {digest:size} to {digest:[size, timestamp]}
- change DiskCache's item file names from "<digest>" to
  "<digest[:2]>/<digest[2:]>"
- introduce VERSION file. On saving, write "2". On loading, delete cache
  dir if it is different from "2".

R=maruel@chromium.org
BUG=654562

[nodir, 2016-10-12 22:33:41]

Description was changed from

==========
isolateserver: DiskCache format v2

- change DiskCache._lru from {digest:size} to {digest:(size, timestamp)}
- change DiskCache's item file names from "<digest>" to
  "<digest[:2]/<digest[2:]>"
- introduce VERSION file. On saving, write "2". On loading, delete cache
  dir if it is different from "2".

R=maruel@chromium.org
BUG=654562
==========

to

==========
isolateserver: DiskCache format v2

- change DiskCache._lru from {digest:size} to {digest:(size, timestamp)}
- change DiskCache's item file names from "<digest>" to
  "<digest[:2]>/<digest[2:]>"
- introduce VERSION file. On saving, write "2". On loading, delete cache
  dir if it is different from "2".

R=maruel@chromium.org
BUG=654562
==========

[nodir, 2016-10-12 22:53:20]

Description was changed from

==========
isolateserver: DiskCache format v2

- change DiskCache._lru from {digest:size} to {digest:(size, timestamp)}
- change DiskCache's item file names from "<digest>" to
  "<digest[:2]>/<digest[2:]>"
- introduce VERSION file. On saving, write "2". On loading, delete cache
  dir if it is different from "2".

R=maruel@chromium.org
BUG=654562
==========

to

==========
isolateserver: DiskCache format v2

- change DiskCache._lru from {digest:size} to {digest:[size, timestamp]}
- change DiskCache's item file names from "<digest>" to
  "<digest[:2]>/<digest[2:]>"
- introduce VERSION file. On saving, write "2". On loading, delete cache
  dir if it is different from "2".

R=maruel@chromium.org
BUG=654562
==========

[nodir, 2016-10-12 22:53:40]

PTAL

[nodir, 2016-10-12 23:24:29]

The CQ bit was checked by nodir@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 23:24:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 23:29:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 23:29:30]

Dry run: Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d407f839faf710)

[M-A Ruel, 2016-10-13 02:19:37]

https://codereview.chromium.org/2414543003/diff/60001/client/isolateserver.py
File client/isolateserver.py (left):

https://codereview.chromium.org/2414543003/diff/60001/client/isolateserver.py...
client/isolateserver.py:1621: self._lru.touch(digest)
Please remove the touch method from LRUDict to be safe.

https://codereview.chromium.org/2414543003/diff/60001/client/isolateserver.py
File client/isolateserver.py (right):

https://codereview.chromium.org/2414543003/diff/60001/client/isolateserver.py...
client/isolateserver.py:8: __version__ = '0.6.0'
bump

https://codereview.chromium.org/2414543003/diff/60001/client/isolateserver.py...
client/isolateserver.py:1510: self.version_file = os.path.join(cache_dir,
self.VERSION_FILE)
I really prefer to keep using a single file, not two. Using two files means
handling the case where one was updated but not the other or whatever
inconsistency that may arises. A quick way to fix this is to remove the methods
load() and save() and move the file I/O back here. Instead add flatten() and
unflatten(data) or something like that. This removes the knowledge about JSON
from LRUDict.

wdyt?

[nodir, 2016-10-13 19:11:14]

Patchset #5 (id:80001) has been deleted

[nodir, 2016-10-13 19:11:28]

The CQ bit was checked by nodir@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 19:11:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 19:16:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 19:16:28]

Dry run: Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d846af60af8310)

[nodir, 2016-10-13 19:18:58]

The CQ bit was checked by nodir@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 19:19:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nodir, 2016-10-13 19:32:42]

I've deleted lru.LRUDict in patchset 5 and now all IO is in the DiskCache. This
looks clean, but...

for named_cache.CacheManager (new thing), I need an ordered dict with timestamps
that can saved to/loaded from a file... which brings me back to
https://codereview.chromium.org/2409103003/

[commit-bot: I haz the power, 2016-10-13 20:20:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 20:20:09]

Dry run: Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31d869871a59f010)

[M-A Ruel, 2016-10-13 20:51:13]

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.py
File client/isolateserver.py (right):

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1589: previous = set(self._lru)
self.cached_set() ? just for the form. :)

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1694: def _cache_disk_size(self):
can you move it aside cache_set? I feel it'd make more sense to have both close.

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1743: with open(self.state_file, 'r') as f:
'rb'

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1746: # Possibly old format.
# Possibly old format. Hard reset with a clear cache.

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1784: with open(self.state_file, 'w') as f:
'wb'

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1789: json.dump(contents, f)
separators=(',',':')

will save a few valuable bytes.

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1839: """Removes the lastest recently used file and
returns its size."""
Probably a typo of mine; latest ?

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1861: Asumes the file exists.
Assumes

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1868: self._lru[digest] = [size, self.time_fn()]
self._lru[digest] = (size, self.time_fn())

it's slightly more memory efficient. It doesn't help once serialized but oh
well.

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1895: if len(os.listdir(parent)) == 0:
I don't think it should be done. It's better to leave the parents there.

https://codereview.chromium.org/2414543003/diff/120001/client/isolateserver.p...
client/isolateserver.py:1896: fs.rmtree(parent)
I don't see a corresponding call to mkdir() to create the two letters parent
directories.

[nodir, 2016-10-13 21:28:23]

On 2016/10/13 19:32:42, nodir wrote:
> I've deleted lru.LRUDict in patchset 5 and now all IO is in the DiskCache.
This
> looks clean, but...
> 
> for named_cache.CacheManager (new thing), I need an ordered dict with
timestamps
> that can saved to/loaded from a file... which brings me back to
> https://codereview.chromium.org/2409103003/

WDYT about this?