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

fix(core): Fix invalid state of accessor.normalized after clone #1275

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

donmccurdy
Copy link
Owner

@donmccurdy donmccurdy commented Feb 19, 2024

Currently the accessor implementation stores references to functions that encode/decode normalized vertex values. When cloning an accessor, those functions held references to their previous scope, and looked up the previous accessor's normalization state.

This PR changes and simplifies the implementation to avoid storing any unnecessary state. The previous implementation was a premature optimization that (as shown by the benchmarks below) did little or nothing. There might be a small regression in join() performance, but that's ongoing work for #1253.

Changes

Benchmark - before

┌─────────┬───────────────┬─────────┬────────────────────┬──────────┬─────────┐
│ (index) │   Task Name   │ ops/sec │ Average Time (ns)  │  Margin  │ Samples │
├─────────┼───────────────┼─────────┼────────────────────┼──────────┼─────────┤
│    0    │  'clone::sm'  │  '78'   │ 12718226.924727235 │ '±2.53%' │   79    │
│    1    │  'clone::md'  │  '18'   │  53303135.9910965  │ '±3.31%' │   19    │
│    2    │ 'create::sm'  │  '135'  │ 7357290.743904955  │ '±3.39%' │   136   │
│    3    │ 'create::md'  │  '33'   │ 30239656.813004438 │ '±3.43%' │   34    │
│    4    │ 'dispose::md' │ '2,063' │ 484532.1858229563  │ '±3.21%' │  2064   │
│    5    │  'join::sm'   │  '33'   │ 30231020.916910734 │ '±4.53%' │   34    │
│    6    │  'join::md'   │   '1'   │ 667896770.6918716  │ '±0.96%' │   10    │
│    7    │  'weld::sm'   │ '1,036' │ 965071.4626192702  │ '±1.77%' │  1037   │
│    8    │  'weld::md'   │  '15'   │  65537336.0812664  │ '±3.01%' │   16    │
└─────────┴───────────────┴─────────┴────────────────────┴──────────┴─────────┘

Benchmark - after

┌─────────┬───────────────┬─────────┬────────────────────┬──────────┬─────────┐
│ (index) │   Task Name   │ ops/sec │ Average Time (ns)  │  Margin  │ Samples │
├─────────┼───────────────┼─────────┼────────────────────┼──────────┼─────────┤
│    0    │  'clone::sm'  │  '78'   │ 12777751.054944873 │ '±2.65%' │   79    │
│    1    │  'clone::md'  │  '18'   │ 52808026.30123339  │ '±2.90%' │   19    │
│    2    │ 'create::sm'  │  '135'  │  7378732.41501133  │ '±3.60%' │   137   │
│    3    │ 'create::md'  │  '33'   │ 29939408.004283905 │ '±2.62%' │   34    │
│    4    │ 'dispose::md' │ '2,084' │ 479733.5695019729  │ '±3.22%' │  2085   │
│    5    │  'join::sm'   │  '28'   │ 35120684.052335806 │ '±4.73%' │   29    │
│    6    │  'join::md'   │   '1'   │ 771483329.2603493  │ '±0.80%' │   10    │
│    7    │  'weld::sm'   │ '1,042' │ 959606.5411974577  │ '±1.80%' │  1043   │
│    8    │  'weld::md'   │  '15'   │ 64910143.24873686  │ '±3.33%' │   16    │
└─────────┴───────────────┴─────────┴────────────────────┴──────────┴─────────┘

@donmccurdy
Copy link
Owner Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@donmccurdy donmccurdy added bug Something isn't working package:core labels Feb 19, 2024
@donmccurdy donmccurdy added this to the v4.0 milestone Feb 19, 2024
@donmccurdy donmccurdy merged commit 773aa45 into main Feb 19, 2024
6 checks passed
@donmccurdy donmccurdy deleted the fix/clone-accessor-normalized branch February 19, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect vertex color dequantization, context-dependent
1 participant