Generic Resource Identifiers with `ResourceIdTrait`

tareknaser commented 4 months ago

Description

This PR aims to make ResourceIndex type generic over the hash function used to generate resource identifiers

Resolves #75

Changes

A new trait is implemented named ResourceIdTrait. This trait defines a set of functions and requirements that all specific ResourceId implementations must implement
Within ResourceIdTrait, a type alias named HashType is defined. This allows different ResourceId implementations to use different types for their hash values, but we ensure that all HashType types implement specific traits for consistent behavior
Modify ResourceIdCrc32 type to implement the ResourceIdTrait
Future work
src/index.rs file heavily relies on the assumption of only using ResourceIdCrc32. It will require adjustments, potentially including abstracting some tests, to function with various ResourceId types.
We can add conditional compilation easily. for example we can use ResourceIdBlake3 that implements ResourceIdTrait
Later, when https://github.com/ARK-Builders/ark-rust/pull/5 is merged, we can move src/resource directory into a separate crate for better organization.

github-actions[bot] commented 4 months ago

Benchmark for a882b55

Click to view benchmark

| Test | Base | PR | % | |------|--------------|------------------|---| | compute_bytes_large/compute_bytes | 141.6±5.35µs | 141.3±3.38µs | -0.21% | | compute_bytes_medium/compute_bytes | 27.5±0.52µs | 27.6±0.42µs | +0.36% | | compute_bytes_small/compute_bytes | **128.9±1.00ns** | 132.7±23.09ns | **+2.95%** | | index_build/index_build/tests/ | **160.9±2.48µs** | 164.1±0.64µs | **+1.99%** | | tests/lena.jpg/compute_bytes | **13.3±0.04µs** | 13.4±0.06µs | **+0.75%** | | tests/test.pdf/compute_bytes | 130.1±3.14µs | **114.1±4.70µs** | **-12.30%** |

tareknaser commented 4 months ago

Take a look at https://github.com/ARK-Builders/arklib/tree/blake3_ResourceIdTrait. It shows how easy it is to include a new ResourceId type that implements ResourceIdTrait, featuring Blake3 as the hash function in this instance. Also notice that ResourceIndex still has collision tracking although blake3 is a cryptographic hash function and there should be no collisions.

github-actions[bot] commented 4 months ago

Benchmark for 1e81c2b

Click to view benchmark

| Test | Base | PR | % | |------|--------------|------------------|---| | compute_bytes_large/compute_bytes | **143.4±0.68µs** | 467.0±2.02µs | **+225.66%** | | compute_bytes_medium/compute_bytes | 28.4±0.20µs | **26.8±0.39µs** | **-5.63%** | | compute_bytes_small/compute_bytes | 134.3±1.00ns | **127.6±0.51ns** | **-4.99%** | | index_build/index_build/tests/ | 166.4±11.56µs | **158.2±1.45µs** | **-4.93%** | | tests/lena.jpg/compute_bytes | 14.0±0.10µs | **13.4±0.16µs** | **-4.29%** | | tests/test.pdf/compute_bytes | 113.3±1.89µs | **110.7±0.66µs** | **-2.29%** |

github-actions[bot] commented 4 months ago

Benchmark for d95e0a5

Click to view benchmark

| Test | Base | PR | % | |------|--------------|------------------|---| | compute_bytes_large/compute_bytes | 487.3±1.14µs | **138.4±2.41µs** | **-71.60%** | | compute_bytes_medium/compute_bytes | 29.1±0.47µs | **28.2±0.93µs** | **-3.09%** | | compute_bytes_small/compute_bytes | 135.0±2.20ns | **127.4±3.10ns** | **-5.63%** | | index_build/index_build/tests/ | 175.9±6.09µs | 174.9±1.20µs | -0.57% | | tests/lena.jpg/compute_bytes | 13.9±0.09µs | **13.3±0.12µs** | **-4.32%** | | tests/test.pdf/compute_bytes | 113.5±0.31µs | **110.1±0.77µs** | **-3.00%** |

ARK-Builders / arklib