{"action":"created","issue":{"url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/334","labels_url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/334/labels{/name}","comments_url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/334/comments","events_url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/334/events","html_url":"https://github.com/Khroki/MCEdit-Unified/issues/334","id":60421470,"number":334,"title":"Problems with \"chunk cache\"","user":{"login":"codewarrior0","id":307683,"avatar_url":"https://avatars.githubusercontent.com/u/307683?v=3","gravatar_id":"","url":"https://api.github.com/users/codewarrior0","html_url":"https://github.com/codewarrior0","followers_url":"https://api.github.com/users/codewarrior0/followers","following_url":"https://api.github.com/users/codewarrior0/following{/other_user}","gists_url":"https://api.github.com/users/codewarrior0/gists{/gist_id}","starred_url":"https://api.github.com/users/codewarrior0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codewarrior0/subscriptions","organizations_url":"https://api.github.com/users/codewarrior0/orgs","repos_url":"https://api.github.com/users/codewarrior0/repos","events_url":"https://api.github.com/users/codewarrior0/events{/privacy}","received_events_url":"https://api.github.com/users/codewarrior0/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-03-09T22:29:28Z","updated_at":"2015-03-09T23:03:21Z","closed_at":null,"body":"Commit 95963c5316981219e8bd2c4f2904d901c97a2538 does not actually add a chunk cache. Chunks are already cached in _loadedChunkData. The purpose of _loadedChunks is to find out via weakrefs whether any clients of MCInfdevOldLevel are holding references to a chunk, to find out if it is safe to evict a cached chunk from _loadedChunkData once the number of chunks exceeds loadedChunkLimit.\r\n\r\nThat commit effectively bypasses the limit by holding an unlimited number of chunks in _loadedChunks.\r\n\r\nThe problem I think you are trying to address is that when chunks are evicted from _loadedChunkData, they are not evicted in any particular order - whichever one comes first in the dict implementation gets evicted. This will clearly lead to \"cache thrashing\" when the most recently loaded chunks are repeatedly evicted and then loaded again. The proper solution is to make _loadedChunkData know which chunks have been least recently used (an LRU cache, a well-known data structure). I had meant to make that change for some time - if you look at https://github.com/Khroki/MCEdit-Unified/blob/master/pymclevel/cachefunc.py, you'll find an LRU cache implementation there. It just has to be adapted into a dict-like data structure (or changed in such a way that when entries are evicted, the chunk can be written to unsavedWorkFolder if needed) instead of one that wraps a function.\r\n\r\nOne problem this commit creates is that MCInfdevOldLevel will not know if it is safe to evict a chunk from the cache. If the cleanup is triggered while any client still has a reference to a chunk, that chunk will be \"lost\" - MCInfdevOldLevel won't know about it any more and any changes made to that chunk won't have any effect. If MCInfdevOldLevel is asked for that chunk again it will return an entirely new AnvilChunk with an entirely new AnvilChunkData behind it. This is precisely the problem that the weakref dict _loadedChunks is meant to solve."},"comment":{"url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/comments/77963391","html_url":"https://github.com/Khroki/MCEdit-Unified/issues/334#issuecomment-77963391","issue_url":"https://api.github.com/repos/Khroki/MCEdit-Unified/issues/334","id":77963391,"user":{"login":"codewarrior0","id":307683,"avatar_url":"https://avatars.githubusercontent.com/u/307683?v=3","gravatar_id":"","url":"https://api.github.com/users/codewarrior0","html_url":"https://github.com/codewarrior0","followers_url":"https://api.github.com/users/codewarrior0/followers","following_url":"https://api.github.com/users/codewarrior0/following{/other_user}","gists_url":"https://api.github.com/users/codewarrior0/gists{/gist_id}","starred_url":"https://api.github.com/users/codewarrior0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codewarrior0/subscriptions","organizations_url":"https://api.github.com/users/codewarrior0/orgs","repos_url":"https://api.github.com/users/codewarrior0/repos","events_url":"https://api.github.com/users/codewarrior0/events{/privacy}","received_events_url":"https://api.github.com/users/codewarrior0/received_events","type":"User","site_admin":false},"created_at":"2015-03-09T23:03:21Z","updated_at":"2015-03-09T23:03:21Z","body":"This all would probably be clearer if _loadedChunks were renamed to _referencedChunks and _loadedChunkData to _cachedChunks"}}