Repository navigation
Conversation
Per-cell get_duration/get_distance go through libosrmc's
osrmc_table_response_duration/distance, which per cell do two
unordered_map lookups, copy a json::Value, and heap-allocate an
osrmc_error for every unreachable cell. For an NxM table that is
N*M FFI calls.
Add a small C++ shim (osrmc-sys/src/osrmc_bulk.{h,cc}), compiled
alongside osrmc.cc, that looks up "durations"/"distances" once and
walks the nested arrays by const reference, writing a row-major float
buffer with INFINITY for null (same unreachable semantics as the
per-cell API). All C++ exceptions are caught at the boundary and
reported via osrmc_error_t, freed by the existing Rust error path.
Rust: table::Response now records num_sources/num_destinations and
exposes durations()/distances() -> Result<Vec<f32>> plus
num_sources()/num_destinations(). The per-cell API is unchanged and
the libosrmc submodule is untouched.
Also drops a trailing blank line in build.rs that failed cargo fmt.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
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.
Context
Ticket: CSIG-3471. This is PR 5, workstream 3: bulk table extraction.
street-routing reads
/api/tableresults one cell at a time, usingResponse::get_durationandget_distance. Each call goes through libosrmc'sosrmc_table_response_durationorosrmc_table_response_distance. For every cell, that means:unordered_map<std::string, Value>lookups (findfollowed byoperator[]),json::Valuevariant,osrmc_errorthat Rust then frees.So an N×M table costs 2·N·M FFI calls. This PR adds one call per matrix.
Changes
osrmc-sys/src/osrmc_bulk.{h,cc}(new). A small C++ shim, compiled next toosrmc.ccinbuild.rs(samecc::Build, flags and includes). It is exposed to bindgen throughwrapper.h.size_t osrmc_table_response_durations(osrmc_table_response_t, float* out, size_t len, osrmc_error_t*)size_t osrmc_table_response_distances(...)"durations"/"distances"once, walks the outer and inner arrays byconst&, and writes a row-major buffer. JSONnullbecomesINFINITY, which matches today's per-cell "unreachable" semantics. It returns the number of cells written.NoTable: the key is missing.BufferTooSmall:lenis too small. It checks before writing anything.Exception: the response has an unexpected shape, or some other C++ exception was thrown. Every exception is caught at the boundary.osrmc_errorusing a definition identical to the one inosrmc.cc. The ODR allows identical definitions, so libosrmc'sosrmc_error_destruct, called from the existing RustError::frompath, frees them correctly.src/table.rs.Responsenow recordsnum_sourcesandnum_destinations, set inOsrm::table.durations()/distances() -> Result<Vec<f32>>: row-major ([from * num_destinations + to]), withf32::INFINITYfor unreachable cells.distances()returns an error when the response was built without distances, likeget_distance.num_sources()/num_destinations()accessors.osrmc-sys/build.rs. Adds one.file(...)for the shim. It also drops a trailing blank line that madecargo fmt --checkfail on master.What's NOT changing
38e6af2, and the shim lives outside it.get_duration/get_distancehave the same signatures and behaviour, includingErrorKind::NoRoutefor unreachable cells.0, which is indistinguishable from a real zero-length route.route,Config/Osrmconstruction, error types and the crate version fields are unchanged.Testing
I ran this in the local
sr-local-ciimage (Debian bullseye, OSRM v5.26.0 built from source, cargo 1.80). The test data came from./prepare-test-data.sh:gcc-states-latestwith the foot profile, extracted with the same OSRM v5.26.0.cargo build: passcargo fmt --all -- --check: passcargo clippy --all-targets -- -D warnings: passcargo test --all: 8 passed, 0 failed. That is the 6 existing tests plus 2 new ones:test_table_bulk_matches_per_cell: a 3×4 non-square table that includes the known unroutableBROKEN_A → BROKEN_Bpair. For every cell, the bulk duration and distance equal the per-cell value exactly. Wherever the per-cell call returnsNoRoute, the bulk value isINFINITY. The test also asserts that the fixture really contains at least one unroutable cell.test_table_bulk_distances_without_distance:distances()returns an error wheninclude_distanceis false, likeget_distance.Note:
Cargo.lockis gitignored in this repo. With cargo 1.80,home 0.5.12(edition 2024, pulled in transitively by bindgen) does not resolve, so I pinned it locally withcargo update -p home --precise 0.5.9. Nothing about this is committed. CI on a newer toolchain is unaffected.Release
After merge, tag
v0.0.3on master. street-routing then bumpsCargo.toml(osrm = { tag = "v0.0.3", ... }) andCargo.lockin a separate street-routing PR, which replaces the per-cell loops incalculate_tablewithdurations()/distances().🤖 Generated with Claude Code