perf(boolean-contains): avoid repeated line scans in LineString check

[rexxars]

isLineOnLine called booleanPointOnLine up to twice per vertex - once for membership and once for the interior probe, and every call re-validated its inputs and allocated an options object, then scanned lineString1 from the first segment.

This PR replaces the calls with a local helper that walks lineString1's raw coordinates once and reports whether a point is off the line or not, only on its start or end vertex, or on its interior. Same segment math as booleanPointOnLine, including the zero-length segment case.

Line/line benchmarks run about 3.1-3.5x faster, e.g. LinesExactlySame goes from 6.5M to 21.5M ops/sec on a MacBook Pro M4.

[rexxars]

@mfedderly I don't love replicating logic from other modules like this, but I don't know if there's a convention for skipping the normalization piece (types signatures would need overloads etc)? How do you feel about multiple exports per package? Eg could we export a named pointOnLineStatus from @turf/boolean-point-on-line, alongside the booleanPointOnLine method, which would return interior | boundary | none? It would need to only take a Position + Position[] to avoid the overhead of normalization (getCoord clones the arrays even if they're the expected shape and does a number of shape matching checks which does cost time). My initial benchmarks say I could get the same perf wins, but also speed up booleanPointOnLine in some cases (by ~20%).

[mfedderly]

Re multiple exports per package: We have this in some places and it can work, but I want to hold the line of "it doesn't get exported unless its a public API surface we intend to support", so we should try to avoid sharing some internal method just for code reuse purposes. I just made this PR as a potential increased API in projection to let people use proj4 but skip the manual handling of the geojson structure itself. Its a bit more lower level than the other two methods, but its related functionality and can be useful for certain use cases.

We've thrown about other ideas for internal code sharing, but most of them wind up with needing to publish a package with an internal API and then hope nobody uses it directly. We could adopt some kind of bundler step, or perhaps some directory of files that are copied to target projects so we introduce duplication in the dists only. I'm not sure if you've got any other ideas on how to code share without making it a public API surface? I was hoping to get away with the entire build step for Turf@v8 just being tsc --build from packages/turf.

I think that some code duplication can work, but the tradeoff comes from how we expect the maintenance burden to work out. We're unlikely to have a problem with two independent implementations of cross-product, but as things grow from there the maintenance gets more painful.

[mfedderly]

I didn't scrutinize the implementation here too closely, but from a high level this seems like duplication that I'd approve and merge if someone said they had a compelling need for the increased performance. As it stands the method is already relatively fast (at least for our test fixtures) and so without someone calling for increased performance we could also get away with just leaving things as is.

At the end of the day someone can always copy/paste the Turf implementation into their own codebase and hyper-optimize it for their exact usage and outperform a more generic implementation. Turf did exactly this for simplify's copy of simplify-js (we use number[] instead of the original {x: number, y: number}.

So I guess I'm curious for feedback from you: is this increased performance useful for you? If so I can take another look here and we can go down the rabbit hole of figuring out a good way to duplicate code across the modules without incurring actual duplication in the code.

[rexxars]

It's not important to me, I was just trying to narrow down/fix any perf regressions in #3011 since you called it out

[mfedderly]

I don't love the regressions in performance, but I'm willing to tolerate them for additional correctness. Everything here is still pretty fast on an absolute scale. Unless its impacting someone I value code clarity and deduplication over the absolute best possible performance.

If its OK with you I'd move to close this PR for now, and if someone feels strongly that the additional performance would benefit them we can revive it and get it landed then.

[rexxars]

We could adopt some kind of bundler step, or perhaps some directory of files that are copied to target projects so we introduce duplication in the dists only. I'm not sure if you've got any other ideas on how to code share without making it a public API surface?

Yes, this is a tough one. Not recommending this, but you could put shared sources in a non-published directory, include them from the consuming package's tsconfig, and set rootDir to the parent. tsc then emits the shared file into each consumer's own dist. The relative import survives and resolves inside dist/, so it'd introduce an additional level of depth to all dists, which is kind of ugly. It also means you'd be duplicating the code across all the modules - might not be the end of the world with gzip'ed bundles, but it kind of rubs me the wrong way.

The other approach would be to just clearly mark private helpers as @private so they are hidden from documentation, and you could potentially prefix-name them, eg _someFn, but nothing stops them from appearing in types/auto-complete etc. You'd need stripInternal: true for that, but it also pulls anything that references a private with it, so that's a no-go.

If its OK with you I'd move to close this PR for now

Sure. I can maybe open a discussion instead where we can talk about perf-related details: I was surprised to find getCoord clones but getCoords don't, and was wondering if there's a reason for that - the cloning costs meaningful performance, so unless we need it, it'd be nice to avoid.

[mfedderly]

👍 sounds good re: discussion. I actually noticed a few days ago that my suggested behavior of "treat geojson as immutable (like React params) and only clone things you actually change" is actually really hard with things like coordEach doing mutation. I'm unsure what I even want to standardize around. I think if you wind up taking a close look you'll find even more inconsistencies around this sort of undocumented behavior.