Single cache file #145
Merged
Single cache file #145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
|
Successfully created a job for commit 9889a9c: |
|
@belltailjp Tests passed. Is this still WIP? |
|
I wanted to try some micro-benchmarks to see if there's non-negligible performance regressions due to increased chance of file access concurrency in |
|
https://gist.github.com/belltailjp/eb6a98a799bf1a56bd42132b92db763b#file-result-md
There are certain amount of fluctuations but I think it's OK to say no significant degradation (nor improvement) in performance. |
belltailjp
force-pushed
the
single-cache-file
branch
from
9889a9c
to
bb85698
Compare
|
/test |
|
Successfully created a job for commit bb85698: |
kuenishi
approved these changes
Feb 16, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently file caches (FileCache and MultiprocessFileCache) save the cache information separately into index file and data file.
This PR is to propose to change it to single file.
The new cache file is formatted as:
where N is the number of samples to cache.
This is actually completely equivalent to just concatenating the current index and data file.
Background
More strict cache size limitation (related to #143)
#143 will introduce limitation of cache data, however currently it is limited to data file only.
It is usually OK - as the index files are relatively negligible in most cases (e.g., ImageNet1K case 34GB vs 20MB), but as the number of parallelism grows the index file increases, which becomes relatively non-trivial, but they'll be out of limitation.
By unifying the index and data file into one file, we can constrain the cache size more strictly.
Easier handling
In case we want to fully utilize cache
preserveandpreloadfunctionality, if the preserved cache is a single file it is easier to handle/manage.Performance concern
@kuenishi has pointed out that since in a single-file cache, every time we access to the cache it reads the file twice (to get index and actually data), which may increase chance of conflict in multi-process use.
To be studied.