AttributeStore memory tweaks #583
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I think AttributeStore lives forever, and AttributeSets are immutable once added to it, so we can avoid the copy.
There are relatively few unique key values for attributes, e.g. `kind`, `name`, `admin_level`. The Shortbread schema has only ~50 or so. I imagine OMT is similar, but haven't checked. We generate lots of AttributePairs -- on the order of tens of millions for GB, and std::string has an overhead of 32 bytes. By using a string pool and storing only an offset into it, we can save a few hundred MB of RAM.
This is the groundwork for implementing two future improvements: - hot/cold pairs: there is a bimodal distribution of attribute frequency. `landuse=wood`, `tunnel=0` are often duplicated. `name=Sneed's Seed & Feed` is not. In the future, we'll try to re-use the "hot" pairs to avoid paying the cost of an AttributePair for them. - "short vectors" - similar to the short string optimization, we should be able to pack up to 6 pairs (3 hot, 3 cold) in the overhead that a vector would otherwise use. As it stands, this commit increases memory usage. But we'll claw a lot of it back, and then some.
If a pair looks like it might be re-usable, put it in a special shard and be able to re-use it. The special shard is limited to max 64K items, teeing up future work to have a simple vector for AttributeSets with few pairs.
The stats I was looking at were counting AttributePairs via AttributeSets, which of course presents a misleading image of how many duplicate AttributePairs there are, because by that point, they've already been deduped. De-duping doesn't add that much runtime overhead--and it could probably be improved by someone who knows more C++ concurrency tricks than me.
`Tile_Value` is a really memory-expensive object. Since we maintain long-lived references to the canonical AttributePair, we can store pointers to save a bit more memory. Now that value->AttributePairs are guaranteed to be 1:1, we can do our debug statistics on ints, and translate to pairs only when writing to stdout.
Doesn't appreciably affect runtime, saves a bit of memory.
Now that there is a 1:1 mapping between values and AttributePairs, it's trivial to compute the hash on demand.
Also const-ify a few things
Tile_Value is a big union that takes up 96 bytes, but for our purposes, we're happy with a union of string, float and bool -- which can be expressed in 28 bytes. We need a discriminator variable, but due to alignment, that's free. I also consider `boost::variant<bool, float, string>`, but it seemed to take 40 bytes. I worried that not having a pool of Tile_Values would affect PBF writing time, but it seems unaffected.
This is useful for ranks, which run from 1..25
`vector<uint32_t>` takes 24 bytes just to store its internal pointers. If you actually want to store a `uint32_t` in it, it'll then allocate some memory on the heap, taking a further 32-64 bytes depending on STL and malloc implementations. 56-88 bytes! For a single `uint32_t`! Outrageous. Instead, store references to pair indexes in an array of shorts. If the pairs don't fit in the array, upgrade it to a vector. Since we previously arranged for very popular pairs like `amenity=toilets` to have small indexes, our array of shorts is capable of storing between 4 and 8 pairs before we need to upgade to a vector. Most AttributeSets will not need to use a vector.
AttributeKeyStore/AttributePairStore have the same lifetime as AttributeStore, so just make them owned by it. This results in slightly more convoluted code, but avoids having them floating around as globals.
x ^ y will only use as many bits as max(x, y), but tiles only use the full 32-bit space at z16, so we're leaving a lot of the hash space on the table.
Previously, if you set the same key to different values, it was not guaranteed that the last value written would win.
set seems a bit like overkill - we already know the items are unique, and the consumer is likely just going to iterate over them
also avoid hardcoding 12
This reverts commit 7570737. Oops, I think this change isn't meaningful, and is a result of me misreading the original code. It might still be an improvement to do something like `hash(x << 16) ^ hash(y)`, since the default TileCoordinate is only 16 bits, but that can be considered independent of this PR.
|
This looks really good - thank you. I'll have a proper look at it this weekend. I like the optimisation for popular pairs - very clever. |
They're long-lived, so pass pointers
I'm slowly remembering how to write concurrent code...
This should reduce futex contention significantly. I'll apply the same change for AttributePairStore's shard 0, then measure.
Per https://stackoverflow.com/questions/36320008/whats-the-default-value-for-a-stdatomic, they aren't initialized by default. Somewhat surprised this didn't result in crashes.
A common pattern is:
```lua
way:Layer("waterway", false)
...
way:Layer("waterway_names", false)
```
Previously, we'd process the geometry twice, and store a second copy of
it in memory. Instead, re-use the previously stored geometry.
This saves another ~1GB of memory for the GB extract. It doesn't
seem to affect runtime - I think we only re-use linestrings, and
linestrings are relatively cheap to do `is_valid` on.
It seems like with the rest of the work on this branch, the
`OutputObjectXyz` classes are very thin -- inspecting `geomType` in
order to construct the right was a bit tedious, so I removed them.
|
Great Britain:
Germany:
That's an amazing improvement. Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a bug where:
sometimes resulted in the first value being saved, sometimes the second. Now the last value always wins.
There are also some memory reductions: about 2,300MB less memory is needed for GB. This comes from two locations:
The PR does a few things.
std::stringinAttributePairto identify the key (name:latin,kind,iata, etc), it identifies the key by auint16_t. This limits Tilemaker to only 64K keys -- probably fine? Shortbread uses about 50, for reference.AttributePairin anAttributeSet, it references them by a vector ofuint32_t. This limits Tilemaker to 4BAttributePairs. This might be an issue -- but it's easily extended to more if needed. I see elsewhere that Tilemaker is limited to 1BAttributeSets -- probably if one has an issue, the other will, too.AttributeSetonly sometimes uses an actual vector ofuint32_ts -- most of the time, a vector is overkill, as items only have a handful of attributes. Instead,AttributeSettypically uses the 24 bytes that a vector would occupy to store an array of 4 shorts and 4 ints†. Only if this array gets exhausted does it change to using a vector.vector_tile::Tile_Valueturns out to be quite big -- it's 96 bytes. I replaced it with a union ofstd::string,floatandbool. I didn't notice any negative impact to PBF generation at the end from newing it up on a per-layer basis.Layerwith the same geometry, and avoid processing/storing the geometry a second time. For example, when a profile writes a geometry for a river as well as for its name.† The system tries to assign IDs that would fit in a short to those
AttributePairs that it thinks will be very popular, likerank=1,amenity=toilets,indoor=falseand so on. This lets mostAttributeSets store their pairs without having to allocate a vector.