⚡ Bolt: [performance improvement] Cache raw bytes instead of parsed dicts in _read_annotations_async - #603
⚡ Bolt: [performance improvement] Cache raw bytes instead of parsed dicts in _read_annotations_async#603sheepdestroyer wants to merge 4 commits into
Conversation
…icts in _read_annotations_async Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR optimizes annotation reads by caching raw JSON bytes and parsing them with Sequence diagram for optimized annotation cache readssequenceDiagram
participant Caller
participant Reader as _read_annotations_async
participant File as AnnotationFile
participant Cache as _annotations_cache
Caller->>Reader: _read_annotations_async(path)
Reader->>Reader: asyncio.to_thread(os.path.getmtime, path)
Reader->>Cache: get(path)
alt cache miss or mtime changed
Reader->>File: aiofiles.open(path, rb)
Reader->>File: read()
File-->>Reader: raw JSON bytes
Reader->>Cache: store mtime and raw_bytes
end
Reader->>Cache: raw_bytes
Reader->>Reader: orjson.loads(raw_bytes)
Reader-->>Caller: parsed annotation dict
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="router/main.py"line_range="4431" />
<code_context>
+ _annotations_cache[path] = {"mtime": current_mtime, "raw_bytes": content}- return copy.deepcopy(_annotations_cache[path]["data"])
+ return orjson.loads(_annotations_cache[path]["raw_bytes"])
</code_context>
<issue_to_address>
**issue (performance):**`orjson.loads(_annotations_cache[path]["raw_bytes"])` now runs synchronously on the event-loop thread, whereas the previous cache-miss path parsed with `asyncio.to_thread`; parsing a large annotation payload therefore blocks other async requests during every cache hit and read.
**Triggers:** When the cached annotation JSON is large or many annotation requests are handled concurrently.
**Suggested fix:** Keep JSON deserialization in `asyncio.to_thread`, or otherwise move the CPU-bound parse off the event loop.
```suggestion return await asyncio.to_thread(orjson.loads, _annotations_cache[path]["raw_bytes"])```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: router/main.py:4431
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| _annotations_cache[path] = {"mtime": current_mtime, "raw_bytes": content} | ||
| return copy.deepcopy(_annotations_cache[path]["data"]) | ||
| return orjson.loads(_annotations_cache[path]["raw_bytes"]) |
There was a problem hiding this comment.
issue (performance):orjson.loads(_annotations_cache[path]["raw_bytes"]) now runs synchronously on the event-loop thread, whereas the previous cache-miss path parsed with asyncio.to_thread; parsing a large annotation payload therefore blocks other async requests during every cache hit and read.
Triggers: When the cached annotation JSON is large or many annotation requests are handled concurrently.
Suggested fix: Keep JSON deserialization in asyncio.to_thread, or otherwise move the CPU-bound parse off the event loop.
| returnorjson.loads(_annotations_cache[path]["raw_bytes"]) | |
| returnawaitasyncio.to_thread(orjson.loads, _annotations_cache[path]["raw_bytes"]) |
…icts in _read_annotations_async Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
…icts in _read_annotations_async Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
…icts in _read_annotations_async Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
💡 What: Replaced caching parsed dictionaries and returning
copy.deepcopy()with caching raw bytes and returningorjson.loads(cached_bytes)in_read_annotations_async.🎯 Why: Returning
copy.deepcopy()from an in-memory cache is slow and blocks the event loop. Storing raw JSON bytes and deserializing on read is significantly faster for returning safe, non-mutating copies.📊 Impact: Eliminates blocking event loop operations caused by
copy.deepcopy(), improving response times for the dashboard annotations API.🔬 Measurement: Benchmark
copy.deepcopy(dict)versusorjson.loads(bytes)for typical annotation payloads to observe the latency reduction.PR created automatically by Jules for task 9129391600238647080 started by @sheepdestroyer
Summary by Sourcery
Optimize asynchronous annotation caching to improve response performance while preserving isolated results for callers.
Enhancements:
Tests:
Chores: