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

JNI wrapper for Rust's BTreeMap #76

Merged
merged 4 commits into from
Jul 6, 2024
Merged

JNI wrapper for Rust's BTreeMap #76

merged 4 commits into from
Jul 6, 2024

Conversation

Pushkarm029
Copy link
Collaborator

@Pushkarm029 Pushkarm029 commented Jun 21, 2024

resolves #70

Copy link

Benchmark for 631e1f4

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 250.1±0.52µs 252.1±1.04µs +0.80%
blake3_resource_id_creation/compute_from_bytes:medium 15.6±0.08µs 15.5±0.08µs -0.64%
blake3_resource_id_creation/compute_from_bytes:small 1355.8±9.89ns 1362.6±7.43ns +0.50%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.8±0.59µs 198.2±0.67µs -0.30%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1748.4±13.56µs 1727.9±19.43µs -1.17%
crc32_resource_id_creation/compute_from_bytes:large 87.1±1.10µs 87.0±0.78µs -0.11%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.07µs 5.4±0.12µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.71ns 92.4±0.43ns 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 66.2±0.50µs 65.6±1.90µs -0.91%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 977.5±7.09µs 975.8±16.10µs -0.17%
index_build/index_build/../test-assets/ 1076.3±7.84µs 1089.7±56.87µs +1.25%

Copy link

Benchmark for 1cd61bc

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 250.1±8.14µs 249.9±1.45µs -0.08%
blake3_resource_id_creation/compute_from_bytes:medium 15.5±0.04µs 15.6±0.05µs +0.65%
blake3_resource_id_creation/compute_from_bytes:small 1373.7±17.69ns 1379.9±23.29ns +0.45%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.1±0.50µs 198.4±1.76µs +0.15%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1727.5±8.41µs 1731.4±20.94µs +0.23%
crc32_resource_id_creation/compute_from_bytes:large 86.8±0.44µs 86.9±0.23µs +0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.07µs 5.4±0.05µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.3±0.21ns 92.9±2.62ns +0.65%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.2±0.27µs 65.4±0.31µs +0.31%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 974.2±6.55µs 974.9±6.14µs +0.07%
index_build/index_build/../test-assets/ 1063.6±50.30µs 1086.7±4.54µs +2.17%

Copy link

Benchmark for b700370

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 247.4±0.94µs 252.2±1.05µs +1.94%
blake3_resource_id_creation/compute_from_bytes:medium 15.6±0.07µs 15.6±0.09µs 0.00%
blake3_resource_id_creation/compute_from_bytes:small 1365.2±8.49ns 1369.1±12.14ns +0.29%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.7±0.56µs 198.8±1.17µs +0.05%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1725.6±3.73µs 1749.9±52.20µs +1.41%
crc32_resource_id_creation/compute_from_bytes:large 86.8±1.30µs 86.7±0.39µs -0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.02µs 5.4±0.03µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.3±0.38ns 92.3±0.35ns 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.7±0.37µs 65.0±0.18µs -1.07%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 978.7±2.88µs 976.0±5.30µs -0.28%
index_build/index_build/../test-assets/ 1056.8±5.07µs 1052.2±6.08µs -0.44%

@Pushkarm029 Pushkarm029 marked this pull request as ready for review June 22, 2024 17:43
Copy link

Benchmark for 2b2d16e

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 250.2±0.82µs 253.1±1.49µs +1.16%
blake3_resource_id_creation/compute_from_bytes:medium 15.6±0.04µs 15.5±0.04µs -0.64%
blake3_resource_id_creation/compute_from_bytes:small 1367.4±2.08ns 1367.4±3.55ns 0.00%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 197.7±0.46µs 197.8±1.24µs +0.05%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1725.1±3.37µs 1730.9±25.47µs +0.34%
crc32_resource_id_creation/compute_from_bytes:large 86.6±0.15µs 86.7±0.15µs +0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.04µs 5.4±0.02µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.42ns 92.3±0.35ns -0.11%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.2±0.36µs 65.2±0.56µs 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 978.7±5.43µs 976.7±2.26µs -0.20%
index_build/index_build/../test-assets/ 1052.6±5.35µs 1056.4±5.76µs +0.36%

@Pushkarm029 Pushkarm029 requested review from tareknaser, twitu and kirillt and removed request for twitu June 23, 2024 05:25
@Pushkarm029
Copy link
Collaborator Author

resolving merge conflicts

Copy link
Collaborator

@twitu twitu left a comment

Choose a reason for hiding this comment

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

We want to wrap the underlying BTreeMap in the FileStorage. We want the BTreeMap to be accessible independent of the FileStorage.

This will most likely lead to a design where the BTreeMap is wrapped like Rc<RefCell<BTreeMap>>. This can then be put in a separate struct called

struct WrapperBTreeMap {
   data: Rc<RefCell<BTreeMap>>
}

We can then expose Java get and iterator functions for this.

The current approach is tightly coupled with the FileStorage implementation and will not scale to other BaseStorage implementations like FolderStorage and ChunkedStorage.

Note: That Rc RefCell will work with us till we add async at which points we'll switch to Arc Mutex.

Copy link

Benchmark for 81d2ac1

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 252.9±4.86µs 248.2±0.89µs -1.86%
blake3_resource_id_creation/compute_from_bytes:medium 15.5±0.10µs 15.6±0.18µs +0.65%
blake3_resource_id_creation/compute_from_bytes:small 1350.4±13.50ns 1368.0±8.83ns +1.30%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 197.8±1.05µs 198.2±1.36µs +0.20%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1724.8±3.43µs 1742.5±69.29µs +1.03%
crc32_resource_id_creation/compute_from_bytes:large 86.9±0.69µs 86.8±0.25µs -0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.03µs 5.4±0.02µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.5±1.47ns 92.4±0.36ns -0.11%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.3±1.87µs 67.4±0.82µs +3.22%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 970.3±2.70µs 985.3±5.19µs +1.55%
index_build/index_build/../test-assets/ 1088.9±12.73µs 1055.1±5.01µs -3.10%

Copy link

Benchmark for cca9b6e

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 250.1±0.54µs 249.2±0.63µs -0.36%
blake3_resource_id_creation/compute_from_bytes:medium 15.5±0.03µs 15.6±0.10µs +0.65%
blake3_resource_id_creation/compute_from_bytes:small 1367.5±7.23ns 1355.3±9.91ns -0.89%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.1±0.57µs 198.3±0.45µs +0.10%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1729.3±3.26µs 1729.9±14.25µs +0.03%
crc32_resource_id_creation/compute_from_bytes:large 86.9±0.54µs 87.0±0.26µs +0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.03µs 5.4±0.03µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.5±1.04ns 92.4±0.45ns -0.11%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.1±0.23µs 65.2±0.79µs +0.15%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 976.3±2.66µs 971.2±3.22µs -0.52%
index_build/index_build/../test-assets/ 1045.1±2.13µs 1044.8±4.48µs -0.03%

Copy link

Benchmark for dd5700d

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 252.0±0.54µs 249.2±1.70µs -1.11%
blake3_resource_id_creation/compute_from_bytes:medium 15.6±0.07µs 15.5±0.06µs -0.64%
blake3_resource_id_creation/compute_from_bytes:small 1352.9±7.08ns 1364.8±38.20ns +0.88%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 197.9±0.90µs 198.0±2.18µs +0.05%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1729.9±18.87µs 1743.7±26.47µs +0.80%
crc32_resource_id_creation/compute_from_bytes:large 86.9±0.32µs 86.9±0.88µs 0.00%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.02µs 5.4±0.06µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.5±1.08ns 92.4±0.94ns -0.11%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 64.8±0.41µs 64.9±0.75µs +0.15%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 973.8±10.87µs 1014.2±3.96µs +4.15%
index_build/index_build/../test-assets/ 1043.6±7.57µs 1045.2±3.74µs +0.15%

@Pushkarm029 Pushkarm029 marked this pull request as draft June 26, 2024 17:59
Signed-off-by: pushkarm029 <[email protected]>
Copy link

Benchmark for 8e69a0f

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 248.5±0.62µs 249.8±1.11µs +0.52%
blake3_resource_id_creation/compute_from_bytes:medium 15.6±0.04µs 15.6±0.12µs 0.00%
blake3_resource_id_creation/compute_from_bytes:small 1363.1±8.87ns 1364.3±7.33ns +0.09%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.9±0.39µs 198.7±0.45µs -0.10%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1779.6±5.39µs 1787.4±20.37µs +0.44%
crc32_resource_id_creation/compute_from_bytes:large 87.0±0.66µs 86.9±0.37µs -0.11%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.02µs 5.5±0.48µs +1.85%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.38ns 92.3±0.25ns -0.11%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.4±0.42µs 65.3±0.18µs -0.15%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 978.4±3.06µs 984.0±4.19µs +0.57%
index_build/index_build/../test-assets/ 1056.3±5.11µs 1090.1±27.38µs +3.20%

Signed-off-by: pushkarm029 <[email protected]>
@Pushkarm029 Pushkarm029 marked this pull request as ready for review June 30, 2024 19:06
Signed-off-by: pushkarm029 <[email protected]>
Copy link

Benchmark for 750678c

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 247.6±0.53µs 249.2±0.48µs +0.65%
blake3_resource_id_creation/compute_from_bytes:medium 15.8±0.06µs 15.8±0.15µs 0.00%
blake3_resource_id_creation/compute_from_bytes:small 1387.4±5.57ns 1386.0±3.40ns -0.10%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 198.0±0.86µs 198.1±0.50µs +0.05%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1734.8±9.99µs 1726.6±15.18µs -0.47%
crc32_resource_id_creation/compute_from_bytes:large 86.8±0.76µs 86.8±0.36µs 0.00%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.03µs 5.4±0.02µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.44ns 92.4±0.56ns 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.5±0.41µs 65.6±0.70µs +0.15%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 953.4±3.78µs 956.6±3.65µs +0.34%
index_build/index_build/../test-assets/ 1046.4±31.86µs 1063.3±4.54µs +1.62%

@Pushkarm029 Pushkarm029 requested a review from twitu June 30, 2024 19:16
Copy link

Benchmark for f4761ef

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 249.8±2.10µs 249.3±1.12µs -0.20%
blake3_resource_id_creation/compute_from_bytes:medium 15.9±1.00µs 15.8±0.13µs -0.63%
blake3_resource_id_creation/compute_from_bytes:small 1386.9±2.70ns 1403.5±11.08ns +1.20%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 197.4±0.39µs 198.3±1.18µs +0.46%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1734.4±13.83µs 1741.2±30.48µs +0.39%
crc32_resource_id_creation/compute_from_bytes:large 87.0±0.54µs 87.5±2.74µs +0.57%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.05µs 5.4±0.03µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.50ns 92.4±0.84ns 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.6±0.88µs 65.1±0.38µs -0.76%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 954.6±5.45µs 962.8±9.13µs +0.86%
index_build/index_build/../test-assets/ 1041.9±18.13µs 1062.1±7.83µs +1.94%

Copy link
Collaborator

@tareknaser tareknaser left a comment

Choose a reason for hiding this comment

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

I've left a couple of comments, but it looks good overall.
Thank you.

fs-storage/src/base_storage.rs Outdated Show resolved Hide resolved
fs-storage/src/jni/btreemap_iter.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@twitu twitu left a comment

Choose a reason for hiding this comment

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

Looks good to merge once @tareknaser's comments are resolved.

@Pushkarm029
Copy link
Collaborator Author

ok, working on it

Signed-off-by: pushkarm029 <[email protected]>
Copy link

github-actions bot commented Jul 4, 2024

Benchmark for bb4aa20

Click to view benchmark
Test Base PR %
blake3_resource_id_creation/compute_from_bytes:large 251.7±1.68µs 251.0±1.02µs -0.28%
blake3_resource_id_creation/compute_from_bytes:medium 15.7±0.03µs 15.8±0.03µs +0.64%
blake3_resource_id_creation/compute_from_bytes:small 1389.0±3.09ns 1383.8±17.43ns -0.37%
blake3_resource_id_creation/compute_from_path:../test-assets/lena.jpg 197.7±1.31µs 198.0±0.65µs +0.15%
blake3_resource_id_creation/compute_from_path:../test-assets/test.pdf 1747.9±4.62µs 1745.7±14.22µs -0.13%
crc32_resource_id_creation/compute_from_bytes:large 86.9±1.28µs 86.8±0.75µs -0.12%
crc32_resource_id_creation/compute_from_bytes:medium 5.4±0.01µs 5.4±0.03µs 0.00%
crc32_resource_id_creation/compute_from_bytes:small 92.4±0.32ns 92.4±0.50ns 0.00%
crc32_resource_id_creation/compute_from_path:../test-assets/lena.jpg 65.8±0.30µs 65.5±0.32µs -0.46%
crc32_resource_id_creation/compute_from_path:../test-assets/test.pdf 962.0±2.34µs 959.5±3.47µs -0.26%
index_build/index_build/../test-assets/ 1071.1±7.54µs 1079.3±53.35µs +0.77%

@twitu twitu merged commit ddec4cf into main Jul 6, 2024
4 checks passed
@Pushkarm029 Pushkarm029 deleted the btreemap branch July 6, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a JNI wrapper for Rust's BTreeMap
3 participants