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

Inconsistent implementation between turn_indexes_write_buffer and turn_weight_penalties/turn_duration_penalties #5862

Closed
wangyoucao577 opened this issue Oct 20, 2020 · 3 comments · Fixed by #5868

Comments

@wangyoucao577
Copy link
Contributor

All the index/weight/duration of turns are generated and written into file in void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges. However, their implementation are a little bit different.

Both turn_weight_penalties and turn_duration_penalties are vectors that cross the function, and will be written into file once when the whole processing finished.

// Now, renumber all our maneuver overrides to use edge-based-nodes
std::vector<StorageManeuverOverride> storage_maneuver_overrides;
std::vector<NodeID> maneuver_override_sequences;

// write weight penalties per turn
BOOST_ASSERT(turn_weight_penalties.size() == turn_duration_penalties.size());
files::writeTurnWeightPenalty(turn_weight_penalties_filename, turn_weight_penalties);
files::writeTurnDurationPenalty(turn_duration_penalties_filename, turn_duration_penalties);

By contrast, turn_indexes_write_buffer will be written to file per 1000. It leads to the write to file action dispersedly in a few places.

storage::tar::FileWriter turn_penalties_index_file(
turn_penalties_index_filename, storage::tar::FileWriter::GenerateFingerprint);
turn_penalties_index_file.WriteFrom("/extractor/turn_index", (char *)nullptr, 0);

// Because we write TurnIndexBlock data as we go, we'll
// buffer them into groups of 1000 to reduce the syscall
// count by 1000x. This doesn't need much memory, but
// greatly reduces the syscall overhead of writing lots
// of small objects
std::vector<lookup::TurnIndexBlock> turn_indexes_write_buffer;
turn_indexes_write_buffer.reserve(TURN_INDEX_WRITE_BUFFER_SIZE);

// Buffer writes to reduce syscall count
if (turn_indexes_write_buffer.size() >= TURN_INDEX_WRITE_BUFFER_SIZE)
{
turn_penalties_index_file.ContinueFrom("/extractor/turn_index",
turn_indexes_write_buffer.data(),
turn_indexes_write_buffer.size());
turn_indexes_write_buffer.clear();
}

After go through the codes, I think unify the process of turn_indexes same as turn_weight_penalties/turn_duration_penalties should be ok, right? Any concern?

@akashihi
Copy link
Contributor

(I haven't checked the actual code) It could be saved in batches due to ram limits, which is not a concern in 2020:) so worth trying in my opinion.

@wangyoucao577
Copy link
Contributor Author

@akashihi I have file a PR, please help to review. thanks!

@akashihi
Copy link
Contributor

akashihi commented Nov 9, 2020

@wangyoucao577 Done, sorry for delay :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants