CacheDataset shared cache issues

[myron]

Due to the changes introduced in the PR:
#5630

it became very inconvenient to use shared cache, and leads to potential crashes

previously, we introduced shared cache (via ListProxy) to speed up training and validation workflows significantly for users during multi-gpu training (with a minimal user changes, by simply setting CacheDataset(runtime_cahe=True))
#5365

with changes merged in PR #5630, all that simplicity was removed, and shared cache allocation and management is left to a user. A user needs to allocate it, and synchronize between processes, and even manually set it to be of proper length.

Internally CacheDataset assigns self.cache_num to keep track of a number of cached elements. Due to PR #5630, there is disconnect between self.cache_num and self._cache, they are not of the same length, and a user doesn't know the self.cache_num to allocate a proper length.

the new (and only way to use shared cache is) CacheDataset(runtime_cache = list_proxy)

1. bug: len(list_proxy) == 0, and self.num_cache > 0, crash
2. bug: len(list_proxy) < self.num_cache , crash
3. potential bug: len(list_proxy) > self.num_cache, disconnect in length, can lead to unforeseen bugs in the future
4. major inconvenience: a User needs to allocate cache manually as Manager().list() in master process and pass it to children. OR a user needs to allocate it in child process and use manual broadcasting to synchronize. Then ensure a proper length. All these steps will be same for all users and use cases. So now, there will be much more redundant coding every time someone wants to use shared memory caching.

[wyli]

Thanks for reporting the issue, I doubt that initializing a listproxy from one of the subprocesses and broadcasting it to the others is a good design. Because of this approach, we have tried to do all the guessings about the user system setup, and have had tricky bugs of the shared cache being garbage collected too early (#5573; #5595; #5565) and inconsistent multiprocessing authkeys (#5613; #5615). I'm sure there will be more of them if we go down this path.

The root cause is that in the distributed training cases, the cachedataset itself is created within subprocesses and shouldn't be responsible for maintaining a global cache shared among subprocesses. So, it's reasonable to require the cachedataset/distributed training users to prepare a suitable cache before creating the subprocess cachedatasets.
Workaround such as #5632 tries to shift the responsibility back into the cachedataset component which in my opinion should be avoided.

[myron]

Thank you for the reply, a good design is also to simplify user workload, and provide API to minimize user efforts, instead of asking to created repeated code blocks. It is generally a responsibility of a library to take care of heavy lifting, even if it's complicated in case of distributed training.

Now that we spent a significant amount of time making it work #5365, this recent the PR #5630 removed the shared memory logic. As it stands now, at a minimum we should simplify it for a user to manage the correct cache length as in #5632. This what a user would have to do manually every time and it also prevents a crash. It is reasonable to simplify user work, and make a safer API.