On older machines loading high portal dense areas this check is
using 70-90% of CPU in Object.keys(cache).length, leading to
resource starvation, and refresh times up to 10 times longer.
The LRU cache high-water mark is checked every time we receive portal
state from the intel servers to see if we need to do GC. In
my case this peaks out 500-1000 portal adds per second on
an aging laptop with the existing code with chrome-unstable,
and minimal IITC plugins. GC itself (ignoring v8 GC) was only
taking 15ms, amortized to about 15usec a portal.
Unforutnately the Object.keys(cache).length check was taking
2-3ms each portal add, and 70-90% of the main thread was spent
there, grinding older machines to a halt.
I've just added a counter to track adds to the cache, and am
updating it with the real cache size on GC (the two checks
are almost always identical in higher density areas, and
won't lead to performance issues if cache adds are over-
estimated).
Effects:
- pushPortalGuidPositionCache goes from using the majority
of CPU when loading dense areas to almost nothing.
- Older machines can actually view western europe without
grinding the browser to a halt.
- the cache itself is now hitting GC_LIMIT in less than
100ms on my old laptop. (I think increasing GC_LIMIT
to 80,000 and GC_KEEP to 40,000 is going to put less
load on the Javascript engine GC, and have the cache
be useful still, but I left that out of the PR as it's
slightly more subjective)
- v8's GC is being slammed slightly less often than normal
(it's still thrashing, but this isn't the main culprit)
The patch has been tested by about half a dozen people
using different blends of IITC plugins on both Chrome,
Firefox and across different OSs and with very different
hardware. Reports ranged from minor to (more often) huge
improvements in load speed and responsiveness in high
density areas.
- search: remove tooltip from DOM immediately instead of setting up a new instance (which would have other options)
- sidebar toggle: remove tooltip upon click, tooltip might persist otherwise
- close other tooltips before opening a new one
- reduce the expiry time (the niantic servers are no longer as unreliable as they were, so keeping old data is less useful than it once was)
- add upper size limit on cache, as well as item count limit
- add debug method to get cache size as a string
in theory, keeping the javascript objects directly in the cache is a good thing. any instances of, for example, portals on the map, will share the data objects with those in the cache, limiting the overheads for cached items in use
however, in practice
- relatively large and complex data structures for cached data. and as some items expired from the cache may have data in live portals, etc, complex for garbage collection to clean up. strings, on the other hand - one single object to clean, zero references from anything else
- the cache is used as an alternative to network requests. therefore the extra time to encode/parse the strings is no real issue
- lower memory overheads? it's liekly a single string is more efficient on RAM use, even taking into account that some objects will be both encoded in the string, and duplicated in live entities on the map
time will tell if this is better or worse than direct object storage...
seems OK, so remove the user prompt - it will break badly if declined. logging remains, to help diagnose issues
also fix silly typo that prevented it from actually working right :)
