sozo: make inspect faster #3056
Conversation
|
Ohayo, sensei! Here’s the update summary: WalkthroughThis update refines the logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant W as WorldLocal::from_directory
participant FS as File System
participant JSON as serde_json::from_slice
U->>W: Call from_directory(dir, profile_config)
W->>FS: List files in directory
loop For each file
W->>W: Check if file ends with ".sierra.json"
alt File ends with ".sierra.json"
W->>W: Log trace and skip file
else
W->>FS: Read file content into bytes
W->>JSON: Deserialize file content
alt Resource is a library file
W->>W: Set dojo_resource_found flag to true
end
end
end
W->>U: Return WorldLocal instance
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
notV4l
force-pushed
the
fast_sozo_inspect
branch
from
702d712 to
8b3683a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/dojo/world/src/local/artifact_to_local.rs (2)
169-171: Consider moving the flag update, sensei.The
dojo_resource_foundflag update could be moved before theelseblock to maintain consistency with other resource types where the flag is set immediately after adding resources.- } else { - dojo_resource_found = true; - } + dojo_resource_found = added_resources; + }
30-364: Consider adding error metrics for better observability, sensei.While the performance improvements are great, consider adding metrics to track:
- Number of skipped
.sierra.jsonfiles- Time spent in JSON deserialization
- Number of resources processed
This would help monitor the performance improvements in production.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/dojo/world/src/local/artifact_to_local.rs(4 hunks)
🔇 Additional comments (3)
crates/dojo/world/src/local/artifact_to_local.rs (3)
49-52: Ohayo! Nice optimization to skip redundant files, sensei!The addition of this check to skip
.sierra.jsonfiles is a great performance improvement, as it prevents unnecessary processing of duplicate data.
54-56: Excellent performance optimization usingfrom_slice, sensei!Switching from
from_readertofrom_sliceis a significant performance improvement. Reading the entire file into memory first and then deserializing from a byte slice is generally faster than reading and parsing incrementally from a file handle.Also applies to: 64-66
371-371: Consistent use offrom_slicehere too, sensei!Good consistency in using
from_slicefor JSON deserialization incasm_class_hash_from_sierra_fileas well.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3056 +/- ##
=======================================
Coverage 57.65% 57.65%
=======================================
Files 439 439
Lines 59437 59442 +5
=======================================
+ Hits 34267 34272 +5
Misses 25170 25170 ☔ View full report in Codecov by Sentry. |
around 10x faster in my case (from 64s to 6s)
Summary by CodeRabbit
Summary by CodeRabbit