Reinitialize UsedObjects when enabling#897
Open
Symmetricity wants to merge 1 commit into
Open
Conversation
UsedObjects::clear frees its node tracking storage after the nodes phase. When reading multiple PBF inputs, the same OSMStore can enable usedNodes again for the next input. set() and test() expect the outer chunk table to exist, so recreate that table when enabling if it was previously cleared. Swap with an empty vector when clearing so the outer chunk table allocation is released rather than retained as capacity.
This file contains hidden or 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
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 PR is AI generated.
Summary
This reinitializes
UsedObjectsstorage when it is enabled after being cleared.UsedObjects::clear()frees the node-tracking storage after the nodes phase toreduce memory use. When tilemaker reads multiple input PBFs in one process, the
same
OSMStoreinstance can then enableusedNodesagain for the next input.UsedObjects::set()andUsedObjects::test()both assume the outer chunk tableexists, so re-enabling after
clear()can otherwise leave those methodsindexing an empty vector.
Background
usedNodesis disabled by default and is only enabled when the Lua profiledeclares
way_keys.The read lifecycle is:
PbfProcessor::ReadPbfFile()callsosmStore.usedNodes.enable()beforeWayScanwhenway_keysare enabled.ScanRelations()andScanWays()can populateusedNodes.ReadNodes()testsusedNodesto decide which nodes to keep.ReadPhase::Nodes,osmStore.usedNodes.clear()frees the storage.happen again on the same
UsedObjectsinstance.Before this patch, step 5 only set the status back to enabled. It did not
restore the outer chunk table that
clear()had removed.Implementation
The patch is intentionally small:
UsedObjects::enable()recreates the outer chunk table if it was previouslycleared.
UsedObjects::clear()swaps with an empty vector so the outer table capacityis released rather than retained.
The hot
set()andtest()paths are unchanged.Why This Location
The reinitialization belongs in
enable()because that is the lifecycleboundary where a previously cleared
UsedObjectsbecomes active again.Adding recovery checks to
set()ortest()would also avoid the crash, butthose are hot paths used while parsing. Keeping the check in
enable()makesthe fix cheaper and keeps the normal lookup/update paths unchanged.
clear()remains the right place to release the memory because it is calledafter
ReadPhase::Nodes, once the node-tracking table is no longer needed forthat input.
Possible Regressions
The intended behavior change is limited to multiple-PBF runs where
usedNodesis enabled again after being cleared.
Expected behavior:
UsedObjectsbehavior is unchangedset()andtest()semantics are unchanged once enabledre-enabled
The patch does not address broader multi-PBF state questions such as whether
relation-scan state should be reset or retained between inputs. It only fixes
the
UsedObjectslifecycle after its storage has been deliberately cleared.Testing
Recommended checks:
git diff --check origin/master..fix/reinitialize-used-objects cmake -S . -B local-builds/reinitialize-used-objects -DCMAKE_BUILD_TYPE=RelWithDebInfo cmake --build local-builds/reinitialize-used-objects --target tilemaker -j2Manual reproduction should use at least two input PBFs with an OpenMapTiles-like
profile that declares
way_keys, sousedNodesis enabled for the first input,cleared after its nodes phase, and then enabled again for the second input.