Skip to content

Conversation

@Turtelll
Copy link
Collaborator

This PR introduces fastPfor decoding to typesctipt

  • testcoverage is reached with unit
  • i tested the decoder with a large fastPfor encoded tileset and re-generated the expected testdata and tests pass

Benedikt Vogl added 23 commits October 20, 2025 13:37
@Turtelll Turtelll self-assigned this Nov 19, 2025
@Turtelll Turtelll added the javascript Pull requests that update Javascript code label Nov 19, 2025
@Turtelll Turtelll requested a review from HarelM November 19, 2025 12:05
@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2025

Codecov Report

❌ Patch coverage is 84.36019% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.80%. Comparing base (d01d7fd) to head (8f449aa).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
ts/src/decoding/fastPforDecoder.ts 91.25% 14 Missing ⚠️
ts/src/decoding/integerDecodingUtils.ts 71.42% 6 Missing and 2 partials ⚠️
ts/src/decoding/integerStreamDecoder.ts 0.00% 6 Missing and 1 partial ⚠️
ts/src/decoding/stringDecoder.ts 0.00% 2 Missing ⚠️
ts/src/decoding/decodingUtils.ts 90.90% 1 Missing ⚠️
ts/src/mltDecoder.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #714      +/-   ##
==========================================
+ Coverage   83.35%   83.80%   +0.44%     
==========================================
  Files          57       58       +1     
  Lines        2439     2605     +166     
  Branches      554      588      +34     
==========================================
+ Hits         2033     2183     +150     
- Misses        380      394      +14     
- Partials       26       28       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@mactrem mactrem left a comment

Choose a reason for hiding this comment

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

Good first draft!

Let's add benchmarks via Benchmark.js to compare the decoding performance with varint encoding based on different datasets

import IntWrapper from "./intWrapper";

// encoded data was generated using the java fastPfor encoder and expected values are re-generated in the tests
const ENCODED_NON_ALINGED_358_ENCODED = new Uint8Array([0, 0, 1, 0, 0, 0, 0, 65, 3, 2, 1, 0, 7, 6, 5, 4, 11, 10, 9, 8, 15, 14, 13, 12, 19, 18, 17, 16, 23, 22, 21, 20, 27, 26, 25, 24, 31, 30, 29, 28, 35, 34, 33, 32, 39, 38, 37, 36, 43, 42, 41, 40, 47, 46, 45, 44, 51, 50, 49, 48, 55, 54, 53, 52,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add the FastPfor test data as binary files to keep the tests more clean.
Also let's add more test cases to ensure that the implementation is also valid for edge cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i switched to using binary files and will add more tests

const hasTrailingBytes = byteLength % 4 !== 0;
const numInts = hasTrailingBytes ? numCompleteInts + 1 : numCompleteInts;

const intValues = new Int32Array(numInts);
Copy link
Collaborator

@mactrem mactrem Nov 19, 2025

Choose a reason for hiding this comment

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

there are two copies generated (data. slice and intValues) before decoding the actual data. Let's reduce the number of data transformations, because at least we do not need slice. Additionaly, if the offset into the data buffer is aligned (to 4 bytes) no copy is needd at all. One approach to complety avoid the copies is to directly work on the DataView but which leads to a slower data access performance see https://v8.dev/blog/dataview. But first let's add benchmarks (against varint as baseline) and then let's optimize

@Turtelll
Copy link
Collaborator Author

Turtelll commented Nov 20, 2025

I will start benchmarking the decoder with different tilesets i encoded accordingly. But i'm not sure if we want to have those benchmarks in the repo itself.

@Turtelll Turtelll changed the title Fast pfor Add fastPfor decoder to Typescript Nov 20, 2025
@mactrem
Copy link
Collaborator

mactrem commented Nov 20, 2025

I will start benchmarking the decoder with different tilesets i encoded accordingly. But i'm not sure if we want to have those benchmarks in the repo itself.

We can think about publishing a seperate package for the fastpfor decoder

@Turtelll Turtelll marked this pull request as draft November 26, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants