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

GEN-196: Apply suggestions from #5181 #5207

Merged
merged 8 commits into from
Sep 23, 2024
Merged

Conversation

indietyp
Copy link
Member

🌟 What is the purpose of this PR?

Implements suggestions from #5181, this needed to be done in another PR, as I didn't want to redo the work done in #5200, which moves the workspace to the new error-stack version.

This is mostly just renames, moving around of methods and documentation improvements.

Open Question:

  • Do we want to default to Panic for ReportSink?

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • modifies a Cargo-publishable library and I have amended the version

📜 Does this require a change to the docs?

The changes in this PR:

  • require changes to docs which are made as part of this PR

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

@github-actions github-actions bot added area/apps > hash* Affects HASH (a `hash-*` app) area/libs > error-stack Affects the `error-stack` crate (library) area/libs Relates to first-party libraries/crates/packages (area) area/libs > deer Affects the `deer` crate (library) type/eng > backend Owned by the @backend team area/tests New or updated tests area/apps area/apps > hash-graph labels Sep 22, 2024
@TimDiekmann TimDiekmann changed the title Apply suggestions from #5181 GEN-196: Apply suggestions from #5181 Sep 23, 2024
@indietyp indietyp force-pushed the bm/error-stack/move-to-v0.6 branch from fcdb39c to e55d2a8 Compare September 23, 2024 09:54
@indietyp indietyp force-pushed the bm/error-stack/apply-suggestions branch from 800bf70 to b053fb5 Compare September 23, 2024 09:55
@indietyp indietyp marked this pull request as ready for review September 23, 2024 09:55
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.52%. Comparing base (6df2a34) to head (635d0d0).
Report is 1039 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5207      +/-   ##
==========================================
- Coverage   19.72%   19.52%   -0.21%     
==========================================
  Files         510      510              
  Lines       17182    17101      -81     
  Branches     2546     2546              
==========================================
- Hits         3390     3339      -51     
+ Misses      13780    13750      -30     
  Partials       12       12              

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

Copy link
Contributor

Benchmark results

@rust/graph-benches – Integrations

scaling_read_entity_linkless

Function Value Mean Flame graphs
entity_by_id 10 entities $$1.88 \mathrm{ms} \pm 6.34 \mathrm{μs}\left({\color{gray}-0.487 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 100 entities $$2.00 \mathrm{ms} \pm 10.6 \mathrm{μs}\left({\color{gray}-1.950 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1000 entities $$2.75 \mathrm{ms} \pm 9.67 \mathrm{μs}\left({\color{lightgreen}-22.426 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10000 entities $$12.6 \mathrm{ms} \pm 156 \mathrm{μs}\left({\color{gray}-2.138 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$1.86 \mathrm{ms} \pm 6.83 \mathrm{μs}\left({\color{gray}-0.238 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_complete_zero_depth

Function Value Mean Flame graphs
entity_by_id 10 entities $$2.08 \mathrm{ms} \pm 10.2 \mathrm{μs}\left({\color{gray}0.078 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 50 entities $$3.79 \mathrm{ms} \pm 20.5 \mathrm{μs}\left({\color{lightgreen}-7.923 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$1.90 \mathrm{ms} \pm 6.48 \mathrm{μs}\left({\color{gray}-1.800 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 25 entities $$2.53 \mathrm{ms} \pm 13.3 \mathrm{μs}\left({\color{lightgreen}-12.377 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$1.89 \mathrm{ms} \pm 11.3 \mathrm{μs}\left({\color{gray}-1.153 \mathrm{\%}}\right) $$ Flame Graph

representative_read_entity

Function Value Mean Flame graphs
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1 $$16.9 \mathrm{ms} \pm 191 \mathrm{μs}\left({\color{gray}-4.826 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1 $$16.2 \mathrm{ms} \pm 186 \mathrm{μs}\left({\color{lightgreen}-36.879 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1 $$15.9 \mathrm{ms} \pm 212 \mathrm{μs}\left({\color{lightgreen}-8.197 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1 $$16.5 \mathrm{ms} \pm 190 \mathrm{μs}\left({\color{lightgreen}-9.288 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1 $$16.8 \mathrm{ms} \pm 180 \mathrm{μs}\left({\color{red}7.06 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1 $$16.7 \mathrm{ms} \pm 184 \mathrm{μs}\left({\color{gray}0.725 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2 $$16.0 \mathrm{ms} \pm 146 \mathrm{μs}\left({\color{gray}-2.676 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1 $$17.2 \mathrm{ms} \pm 189 \mathrm{μs}\left({\color{red}6.59 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1 $$15.7 \mathrm{ms} \pm 141 \mathrm{μs}\left({\color{lightgreen}-12.637 \mathrm{\%}}\right) $$ Flame Graph

representative_read_entity_type

Function Value Mean Flame graphs
get_entity_type_by_id Account ID: d4e16033-c281-4cde-aa35-9085bf2e7579 $$1.41 \mathrm{ms} \pm 2.92 \mathrm{μs}\left({\color{gray}-1.577 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_complete_one_depth

Function Value Mean Flame graphs
entity_by_id 10 entities $$51.4 \mathrm{ms} \pm 172 \mathrm{μs}\left({\color{gray}-1.391 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 50 entities $$276 \mathrm{ms} \pm 2.87 \mathrm{ms}\left({\color{lightgreen}-30.926 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$25.1 \mathrm{ms} \pm 195 \mathrm{μs}\left({\color{gray}-0.466 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 25 entities $$72.2 \mathrm{ms} \pm 306 \mathrm{μs}\left({\color{gray}-4.355 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$19.9 \mathrm{ms} \pm 85.1 \mathrm{μs}\left({\color{gray}-1.977 \mathrm{\%}}\right) $$ Flame Graph

representative_read_multiple_entities

Function Value Mean Flame graphs
link_by_source_by_property depths: DT=2, PT=2, ET=2, E=2 $$99.9 \mathrm{ms} \pm 522 \mathrm{μs}\left({\color{gray}-2.941 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=2, ET=2, E=2 $$94.3 \mathrm{ms} \pm 298 \mathrm{μs}\left({\color{gray}-3.279 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=255, PT=255, ET=255, E=255 $$107 \mathrm{ms} \pm 411 \mathrm{μs}\left({\color{gray}-3.077 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=0 $$41.7 \mathrm{ms} \pm 192 \mathrm{μs}\left({\color{lightgreen}-5.337 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=2 $$80.0 \mathrm{ms} \pm 285 \mathrm{μs}\left({\color{gray}-0.482 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=2, E=2 $$89.7 \mathrm{ms} \pm 346 \mathrm{μs}\left({\color{lightgreen}-5.211 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=2, PT=2, ET=2, E=2 $$58.7 \mathrm{ms} \pm 270 \mathrm{μs}\left({\color{gray}-3.991 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=2, ET=2, E=2 $$55.4 \mathrm{ms} \pm 365 \mathrm{μs}\left({\color{gray}-2.944 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=255, PT=255, ET=255, E=255 $$69.0 \mathrm{ms} \pm 404 \mathrm{μs}\left({\color{gray}1.24 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=0 $$39.3 \mathrm{ms} \pm 243 \mathrm{μs}\left({\color{lightgreen}-5.526 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=2 $$44.3 \mathrm{ms} \pm 176 \mathrm{μs}\left({\color{gray}-2.260 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=2, E=2 $$51.3 \mathrm{ms} \pm 279 \mathrm{μs}\left({\color{gray}1.93 \mathrm{\%}}\right) $$ Flame Graph

@indietyp indietyp enabled auto-merge September 23, 2024 14:27
@indietyp indietyp disabled auto-merge September 23, 2024 14:28
@indietyp indietyp added this pull request to the merge queue Sep 23, 2024
Merged via the queue into main with commit 8441d3d Sep 23, 2024
122 checks passed
@indietyp indietyp deleted the bm/error-stack/apply-suggestions branch September 23, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-graph area/apps area/libs > deer Affects the `deer` crate (library) area/libs > error-stack Affects the `error-stack` crate (library) area/libs Relates to first-party libraries/crates/packages (area) area/tests New or updated tests type/eng > backend Owned by the @backend team
Development

Successfully merging this pull request may close these issues.

2 participants