src/cpp/fastgpx/fastgpx.cpp carries a lone // TODO: Add parse options, selectively choose what to parse. (line 455, just above the <time> read). Several other issues point at it as the
mechanism that would resolve them, but it has no issue of its own, so the design is never actually
discussed. Filing it so it can be.
Why it keeps coming up
The parse wins so far came from reading less per point, not from parsing faster: moving the
<time> string into TimePoint instead of copying it, and std::from_chars instead of strtod
(#23 has the numbers). Every proposal to read another element runs against that, and without a way
to opt out the answer is always either "make everyone pay" or "do not read it at all".
Issues currently blocked on or complicated by the absence of parse options:
So this splits into two independent axes that have been getting conflated:
- What to read. Skip
<time>, skip <ele>, skip track metadata, points only. Saves work.
- How strict to be. Reject malformed coordinates and timestamps, or accept them silently as
today. Costs work.
They probably want to be the same options object, but they are different decisions and #13 only
needs the first.
What is parsed today
Per point, unconditionally: lat, lon, <ele>, and the raw <time> string (parsed lazily on
first access). Per file: <metadata><name>. Nothing else - see #13 for the <trk> children that
are declared but never read.
Design questions
- Shape of the API. An options struct passed to
LoadGpx/ParseGpx is the obvious C++ form.
In Python it would be keyword arguments on load()/parse(), which is friendlier than exposing
a struct, but then the two surfaces drift. Worth deciding once.
- Defaults. Whatever the default is becomes the benchmarked path and the one users see, so the
default matters more than the options. Reading everything by default is the least surprising;
reading the minimum by default is the fastest headline number.
- Granularity. Per-element flags (
parse_time, parse_elevation, parse_track_metadata) are
simple and map onto the code directly. A preset (minimal / default / strict) is friendlier
but has to be defined in terms of the flags anyway.
- Interaction with lazy evaluation.
<time> is already deferred to first access, which is a
form of "do not pay for what you do not use" that costs nothing at parse time. Some of what
options would buy may be better solved that way instead - worth checking per field before adding
a flag for it.
- Cost of the check itself. A per-point
if (options.parse_time) is a predictable branch and
should be free, but this is the parser, so it needs the usual before/after benchmark rather than
an assumption.
Not urgent
Nothing is broken today. This is a prerequisite that other issues keep bumping into, and the
purpose of filing it is so that #13 and the #51/#14/#52 leniency discussion can refer to a real
design rather than a TODO comment.
src/cpp/fastgpx/fastgpx.cppcarries a lone// TODO: Add parse options, selectively choose what to parse.(line 455, just above the<time>read). Several other issues point at it as themechanism that would resolve them, but it has no issue of its own, so the design is never actually
discussed. Filing it so it can be.
Why it keeps coming up
The parse wins so far came from reading less per point, not from parsing faster: moving the
<time>string intoTimePointinstead of copying it, andstd::from_charsinstead ofstrtod(#23 has the numbers). Every proposal to read another element runs against that, and without a way
to opt out the answer is always either "make everyone pay" or "do not read it at all".
Issues currently blocked on or complicated by the absence of parse options:
measurements in that issue suggest the cost is negligible for realistic files, but there is no
mechanism to let a caller decline it if it is not.
option" as one of the three options, explicitly referencing this TODO.
<ele>and unparseablelat/lon.<time>makestime_bounds()throw) - "validate timestamps eagerly at parsetime" is really a parse option.
So this splits into two independent axes that have been getting conflated:
<time>, skip<ele>, skip track metadata, points only. Saves work.today. Costs work.
They probably want to be the same options object, but they are different decisions and #13 only
needs the first.
What is parsed today
Per point, unconditionally:
lat,lon,<ele>, and the raw<time>string (parsed lazily onfirst access). Per file:
<metadata><name>. Nothing else - see #13 for the<trk>children thatare declared but never read.
Design questions
LoadGpx/ParseGpxis the obvious C++ form.In Python it would be keyword arguments on
load()/parse(), which is friendlier than exposinga struct, but then the two surfaces drift. Worth deciding once.
default matters more than the options. Reading everything by default is the least surprising;
reading the minimum by default is the fastest headline number.
parse_time,parse_elevation,parse_track_metadata) aresimple and map onto the code directly. A preset (
minimal/default/strict) is friendlierbut has to be defined in terms of the flags anyway.
<time>is already deferred to first access, which is aform of "do not pay for what you do not use" that costs nothing at parse time. Some of what
options would buy may be better solved that way instead - worth checking per field before adding
a flag for it.
if (options.parse_time)is a predictable branch andshould be free, but this is the parser, so it needs the usual before/after benchmark rather than
an assumption.
Not urgent
Nothing is broken today. This is a prerequisite that other issues keep bumping into, and the
purpose of filing it is so that #13 and the #51/#14/#52 leniency discussion can refer to a real
design rather than a TODO comment.