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

Reduce memory in stxxl #3646

Merged
merged 1 commit into from
Feb 3, 2017
Merged

Reduce memory in stxxl #3646

merged 1 commit into from
Feb 3, 2017

Conversation

oxidase
Copy link
Contributor

@oxidase oxidase commented Feb 3, 2017

Issue

Issue #77 introduced a regression in memory consumption in InternalExtractorEdge. This PR aimed to reduce memory consumption by using float type for the value and avoid the variant type. The float type can be used with assumption that initial edge weights and duration values from profiles have 24 significant bits (about 7 decimal digits). This is pretty safe assumption, because these values are used as 31 significant bits integers and summing up without precision loss 128 maximum possible values will lead to signed integer overflow. To route for over 128 edges without risk of the integer overflow edge weights should have less than 24 significant bits.
The variant type introduces 8 bytes overhead by std::size_t type_index and results to sizeof(mapbox::util::variant<osrm::extractor::detail::ValueByEdge, osrm::extractor::detail::ValueByMeter>)
= 16 bytes per WeightData or 32 bytes per edge. Because there are only two types ValueByEdge that is always non-negative and ValueByMeter that is always strictly positive the sign bit can be used to encode the value type in a single value without values domains overlap.

Structure sizes:

sizeof(osrm::extractor::InternalExtractorEdge::WeightData) = 16
sizeof(osrm::extractor::NodeBasedEdgeWithOSM) = 40
sizeof(osrm::extractor::InternalExtractorEdge) = 64
ptype osrm::extractor::InternalExtractorEdge
type = struct osrm::extractor::InternalExtractorEdge {
    osrm::extractor::NodeBasedEdgeWithOSM result;
    osrm::extractor::InternalExtractorEdge::WeightData weight_data;
    osrm::util::Coordinate source_coordinate;
sizeof(osrm::extractor::InternalExtractorEdge::WeightData) = 16
sizeof(osrm::extractor::NodeBasedEdgeWithOSM) = 48
sizeof(osrm::extractor::InternalExtractorEdge) = 88
ptype osrm::extractor::InternalExtractorEdge
type = struct osrm::extractor::InternalExtractorEdge {
    osrm::extractor::NodeBasedEdgeWithOSM result;
    WeightData weight_data;
    WeightData duration_data;
    osrm::util::Coordinate source_coordinate;
  • with the PR
sizeof(osrm::extractor::InternalExtractorEdge::WeightData) = 4
sizeof(osrm::extractor::NodeBasedEdgeWithOSM) = 44
sizeof(osrm::extractor::InternalExtractorEdge) = 60
ptype osrm::extractor::InternalExtractorEdge
type = struct osrm::extractor::InternalExtractorEdge {
    osrm::extractor::NodeBasedEdgeWithOSM result;
    WeightData weight_data;
    WeightData duration_data;
    osrm::util::Coordinate source_coordinate;

So PR saves 28 bytes per edge wrt to the current master (>11GB) and 4 bytes wrt master before #2399 merge (1.6GB) for raw 392418286 input ways.

Tasklist

  • review
  • adjust for comments

Requirements / Relations

The PR is rebased on top of #3629, so only e6c884d commit is relevant for review.

@oxidase oxidase force-pushed the try/reduce-memory-in-stxxl branch from 0a06ff5 to e6c884d Compare February 3, 2017 08:04
@oxidase oxidase added the Review label Feb 3, 2017
Copy link
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase on master now that #3632 is merged otherwise this looks good to me.

// Use a single float to pack two positive values:
// * by_edge: + sign, value >= 0, value used as the edge weight
// * by_meter: - sign, value > 0, value used as a denominator in distance / value
struct ByEdgeOrByMeterValue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elegant solution. Precision should be more than enough.

@oxidase oxidase force-pushed the try/reduce-memory-in-stxxl branch from e6c884d to 94fb801 Compare February 3, 2017 13:42
@oxidase
Copy link
Contributor Author

oxidase commented Feb 3, 2017

rebased. with the change i see only two lines [STXXL-ERRMSG] External memory block allocation error: 2145386496 bytes requested, 1163919360 bytes free. Trying to extend the external memory space...
on the planet size and extraction + contraction time is 14 hours

@TheMarex TheMarex merged commit c48fc58 into master Feb 3, 2017
@TheMarex TheMarex deleted the try/reduce-memory-in-stxxl branch February 3, 2017 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants