Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item name and map action cache #40

Merged
merged 31 commits into from
Mar 17, 2020

Conversation

youbetterdont
Copy link

@youbetterdont youbetterdont commented Mar 9, 2020

  • Item name cache implemented. Item names are looked up from cache when possible, bypassing the call to evaluate.
  • The item name cache uses the unmodified item name in addition to the modified one. This allows for detecting changes in the unmodified item name. This is used to trigger updates to the modified item name.
  • PrintText will now truncate the string instead of crashing when it's too long.

Here is the profiler result after the name cache implementation:
image

Here's the callstack for Evaluate:
image

The dominant evaluate calls are coming from OnAutoMap draw now rather than GetItemName.

FYI, I left on a print statement that fires whenever a change is detected to the unmodified item name. This is what I use to trigger cache updates. You'll see it go off when IDing items, making runewords, etc. The reason I left it on is that I want to know if it ever fires in other situations, like if you leave an item on the ground and come back to it after a long time. I don't think this is harmful (since it's detected afterall), but it's just interesting to know.

I pulled in an LRU cache implementation as a subtree. That's what a lot of these commits are. It's a header only library, so you'll just need to modify your include path if using the Visual Studio project. The cmake files have already been udpated.

This PR includes the previous performance related PRs.

Sleepy files:
capture_chaos_run_item_name_cache.zip

TODO:

  • Reset cache when reloading config file. I think that the item names won't update when the config is refreshed until the cache happens to get full of other stuff.
  • Remove the 'Reset LRU cache' message that prints when joining games.

Update 3/12:

  • Refactored caching code to make it easier to apply in other places.
  • For items already in the cache, updates are now triggered by changes in the item flags instead of the original item name. This is a more flexible approach (and also detects some things that name doesn't).
  • Added map action cache. This caches the color of items on the automap.

Performance after the map cache:
image

Sleepy:
capture_chaos_after_map_rule_fix.zip

No BH functions are visible at the top anymore!

Notes:

  • Need to make extra triple sure that the cache is always updated when it should be.
  • Cache is keyed by GUID. This is a unique number shared by the client and server.
  • GUIDs are not unique between games, but the caches are reset in OnGameJoin().
  • Cache is also reset when reloading BH config.
  • Cache size is set to 50 100. Least recently used items are removed first.
  • If an item is already in the cache, it will still be updated if item flags change. This is how we deal with IDing items, making runewords, etc.
  • If anyone can think of other corner cases where we might miss a cache update or something that could cause GUIDs to be reused on different items, that would be good to know.

Update 3/17:

  • These changes have been tested by myself and a few slashers. This feature appears to be stable.

lamerman and others added 28 commits June 20, 2013 05:57
Google testing framework added. Tests refactored
Simple reproduction of the bug:
    LRUCache<string, string> lruCache(3);
    lruCache.put("key1", "val1");
    lruCache.put("key2", "val2");
    lruCache.put("key3", "val3");
    lruCache.put("key1", lruCache.get("key1"));
The get returns a ref to the value of "key1", the put gets the ref and
invalidate it when it finds "key1" already exists in the cache.
Download google tests from git with a fixed tag. Svn is no longer
available.
Similar name of cache instance and cache namespace in tests
caused the error: redeclared as different kind of symbol.
Fix redeclared as different kind of symbol issue
Fix wrong namespace name in comment
…ab5c2961411dbe22'

git-subtree-dir: ThirdParty/cpp-lru-cache
git-subtree-mainline: d774da7
git-subtree-split: de1c4a0
…en possible, bypassing the call to evaluate.

* The item name cache uses the unmodified item name in addition to the modified one. This allows for detecting changes in the unmodified item name. This is used to trigger updates to the modified item name.
* PrintText will now truncate the string instead of crashing when it's too long.
@youbetterdont youbetterdont changed the title Item name cache Item name and map action cache Mar 12, 2020
@planqi planqi merged commit 5503716 into planqi:master Mar 17, 2020
@youbetterdont youbetterdont deleted the feature/get-item-name-cache-2 branch March 18, 2020 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants