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

Cache whether a Table is a scheduler table, avoiding fetching the schema #2141

Draft
wants to merge 2 commits into
base: centril/update-abi
Choose a base branch
from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 17, 2025

Description of Changes

Fixes #2021.

Does 2 things:

  1. adds fn update_mut_tx to traits.rs
  2. adds Table::is_scheduler so datastore_{insert, update}_bsatn don't have to fetch the schema.

Perf before:

for i in {1..5}; do spacetime call basics update_positions_by_collect -s local; done

2025-01-17T20:57:06.015709Z  INFO: : Timing span "update_positions_by_collect": 615.027471ms
2025-01-17T20:57:07.248089Z  INFO: : Timing span "update_positions_by_collect": 618.673753ms
2025-01-17T20:57:08.473255Z  INFO: : Timing span "update_positions_by_collect": 607.102709ms
2025-01-17T20:57:09.581249Z  INFO: : Timing span "update_positions_by_collect": 608.032989ms
2025-01-17T20:57:10.787973Z  INFO: : Timing span "update_positions_by_collect": 606.664556ms

Perf after:

for i in {1..5}; do spacetime call basics update_positions_by_collect -s local; done

2025-01-17T22:52:31.389501Z  INFO: : Timing span "update_positions_by_collect": 569.702228ms
2025-01-17T22:52:32.599270Z  INFO: : Timing span "update_positions_by_collect": 585.807116ms
2025-01-17T22:52:33.686554Z  INFO: : Timing span "update_positions_by_collect": 597.645714ms
2025-01-17T22:52:34.893818Z  INFO: : Timing span "update_positions_by_collect": 586.649907ms
2025-01-17T22:52:35.994934Z  INFO: : Timing span "update_positions_by_collect": 585.149919ms

This means we go from an average 610 ms to 585 ms, which is about an average 25 ms speedup.
This was the expected speedup based on % of samples in the update ABI PR flamegraph.

API and ABI breaking changes

None

Expected complexity level and risk

1, fairly simple and contained change.

Testing

TODO

@Centril Centril force-pushed the centril/update-abi branch from 6770f62 to bc2a869 Compare January 21, 2025 04:28
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.

1 participant