api icon indicating copy to clipboard operation
api copied to clipboard

Potential size savings in DestinyInventoryItemDefinition

Open bhollis opened this issue 6 years ago • 14 comments

With all the new items, the manifest is getting very large. From DIM's perspective, we're starting to run into memory issues as a result. This issue is a brainstorm of some of the properties that might be easy to trim out to save some space.

  • DestinyInventoryItem#backgroundColor - not clear what this is even for, but it's often all zeroes - it could just be null.
  • DestinyInventoryItem#action - I'm not aware of any item manager including the Companion that makes use of this info. Especially for equipment, all this is is a very verbose way of saying "Dismantle".
  • DestinyInventoryItem#itemTypeDisplayName could be folded into a DestinyItemTypeDefinition.
  • DestinyInventoryItem#equippingBlock.displayStrings - not clear how these would be used, but they're often an array of a single empty string.
  • DestinyInventoryItem#talentGrid - almost every item has one, but they reference weird empty talent grids. Only subclasses have populated ones. It feels like this could just be null most of the time.

bhollis avatar Oct 19 '19 03:10 bhollis

https://github.com/Bungie-net/api/issues/64 is another bit that could be saved.

bhollis avatar Oct 19 '19 03:10 bhollis

Hey thanks, these are good points. We'll certainly consider them!

jshaffstall-bng avatar Oct 19 '19 04:10 jshaffstall-bng

I would like to add to the awesome suggestions that @bhollis has made: Item duplication.

Ex: 1 Peregrine Greaves - There are two entries for this item (2255796155.0 & 3257252969.0). One has a collectibleHash and the other does not. One has a loreHash and the other does not.

Ex: 2 The Jade Rabbit. There is a total of 8 hashes for this item. 4 of them are Weapons (2896466320.0, 2978016230.0, 3229272315.0, 3844694310.0). Mind you that 3844694310.0 is the only actual item here. The other 4 of them are Exotic Packages (2192598950.0, 2257699986.0, 2602176822.0, 3536086285.0). I have not dug too much into investigating each of the 4 packages, but just glancing over them, they are all seem to contain very similar information.

The two above examples are just instances were some item consolidation could go a long way in gaining a few megs of head room. Just looking at some stats from my local MongoDB instance, the DestinyInventoryItemDefinition collection contains 6583 docs, with a uncompressed size of 24.2 MB. Thankfully, WiredTiger compresses this down to 8 MB. AvgObjectSize is roughly 3.688Kb.

Thanks for all you guys and gals do and keep up the great work!

unisys12 avatar Oct 19 '19 20:10 unisys12

I'm kind of in agreement but also disagreement with some of this.

  • DestinyInventoryItem#backgroundColor - not clear what this is even for, but it's often all zeroes - it could just be null.

I found one item that had RGBA values of 255, but I didn't quite see the purpose of it either.

  • DestinyInventoryItem#action - I'm not aware of any item manager including the Companion that makes use of this info. Especially for equipment, all this is is a very verbose way of saying "Dismantle".

Actually, some items have different verbs like "Use" or "Scrap", which can mean completely different things, so I wouldn't want to see those go. Some of my personal picks to remove from the DestinyInventoryItemDefinition are uiItemDisplayStyle and preview.

I would like to add to the awesome suggestions that @bhollis has made: Item duplication.

Ex: 1 Peregrine Greaves - There are two entries for this item (2255796155.0 & 3257252969.0). One has a collectibleHash and the other does not. One has a loreHash and the other does not.

Ex: 2 The Jade Rabbit. There is a total of 8 hashes for this item. 4 of them are Weapons (2896466320.0, 2978016230.0, 3229272315.0, 3844694310.0). Mind you that 3844694310.0 is the only actual item here. The other 4 of them are Exotic Packages (2192598950.0, 2257699986.0, 2602176822.0, 3536086285.0). I have not dug too much into investigating each of the 4 packages, but just glancing over them, they are all seem to contain very similar information.

I've actually been hoping that some of these weird duplicates would get cleaned up from the API. However, bear in mind that there are actually duplicate weapons in the game spanning different seasons. A notable example being Erentil FR4, which was released in vanilla Destiny 2 (DestinyInventoryItem hash: 2398848320) and re-released in the Forsaken expansion (DestinyInventoryItem hash: 3027844941).

Xalavar avatar Oct 19 '19 21:10 Xalavar

Yeah some dupes are really different items, and there are lots of duplicated and seemingly unnecessary entries in all of the different content tables. But most of these have very specific and subtle purposes that make them difficult to strip out. I would consider it to be creative use of game (design) mechanics.

The item display style can certainly come in handy in some cases, but we could turn that into an enum if we needed to.

And thanks to you all for opening issues. Hopefully we'll be able to get to more of these sooner rather than later.

jshaffstall-bng avatar Oct 19 '19 22:10 jshaffstall-bng

I took a look at the new DestinyInventoryItemLiteDefinition - unfortunately, it omits too much that we need in DIM (sockets, stats, etc). I'm not looking to necessarily lose information - just stuff that provides no value.

I did a quick experiment deleting the properties I suggested above and it shaves 5MB off the InventoryItem definitions, from 54M -> 49M. There's lots of opportunity here.

We're seeing increasing reports of DIM crashing people's browsers as it attempts to download and digest the manifest. Making some reductions here would help a lot.

bhollis avatar Apr 05 '20 05:04 bhollis

Just some more stats - the InventoryItem table occupies 61MB in browser memory, and each individual item takes about 10.5KB. Reducing either the size of items or the number of items would have a big impact.

bhollis avatar Apr 05 '20 20:04 bhollis

The biggest contributor to item size is sockets, which is important to have in DIM for a bunch of features (and is missing in the InventoryItemLite database). Each sockets property contains:

  • A separate copy of the string "Details" - every sockets object has this.
  • Each element of reusablePlugItems is an object with a single key, plugItemHash - if this were a simple array of numbers it would be much smaller.
  • Masterwork-able items include every possible masterwork plug in their reusablePlugItems which contributes ~4.5KB to each item. This is redundant with the reusablePlugSetHash on the item - DIM uses that instead of the list of reusablePlugItems.

The next biggest is stats (1.5KB), which contains:

  • A map of stats, keyed by statHash. The statHash is then repeated in the value.
  • Each object also includes a maximum, minimum, and displayMaximum, all of which are the same value and could be part of the stat definition.
  • This entire thing could be a map from statHash to value.
  • investmentStats (which we use!) could be the same way.

Going back to talentGrid, which I mentioned originally:

  • Another copy of the string "Details"
  • A reference to an empty talent grid definition.

General notes:

  • Believe it or not, localized strings and image paths don't contribute the majority of memory usage for items.
  • That said, strings like "/img/misc/missing_icon_d2.png" don't really need to be repeated over and over.
  • In general, V8 uses 28 bytes to represent any boolean property. Making more use of flags could help.
  • translationBlock, used for 3D rendering, could be broken into a separate table as it's only used if you're trying to render 3D.

bhollis avatar Apr 05 '20 21:04 bhollis

The Collectibles table duplicates most of the strings for the item it references - replacing that with an inventoryItemHash would save a lot of space.

bhollis avatar Apr 05 '20 21:04 bhollis

By trimming some of these things out before saving the manifest, I've managed to reduce the DestinyInventoryItemDefinitions table in-memory footprint to 35MB from 61MB. This doesn't help folks who can't handle the initial parse into memory of 61MB though.

I found some more stuff too - I'm not sure how iconSequences are meant to be used in the general sense, but they seem to contribute a fair amount of bloat, e.g. https://data.destinysets.com/i/InventoryItem:1390870800

bhollis avatar Apr 05 '20 23:04 bhollis

It turns out trimming after parsing has a lot of pitfalls - the V8 implementation of JSON parsing makes a lot of optimizations that get undone once you mess with the result object. We can make a lot of progress, but having the JSON in better shape in the first place would really be the best.

bhollis avatar Apr 05 '20 23:04 bhollis

I found some more stuff too - I'm not sure how iconSequences are meant to be used in the general sense, but they seem to contribute a fair amount of bloat, e.g. https://data.destinysets.com/i/InventoryItem:1390870800

That seems like a big dud but sequences are very important for metrics. I'm not fond of them though

justrealmilk avatar Apr 06 '20 01:04 justrealmilk

Icon sequences are fine for metrics but I’m not clear what structure they follow, and for most items they’re not valuable beyond the normal icon.

bhollis avatar Apr 06 '20 01:04 bhollis

This is still important - the manifest size is really hurting us in terms of memory usage, especially on buggier browsers like Firefox.

bhollis avatar Dec 13 '24 19:12 bhollis