perf: copy-on-write for sentry_value_t clone#1794
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec86d1f. Configure here.
jpnurmi
changed the title
jpnurmi
force-pushed
the
jpnurmi/ref/copy-on-write
branch
from
ec86d1f to
3e7dd7b
Compare
jpnurmi
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
jpnurmi
force-pushed
the
jpnurmi/ref/copy-on-write
branch
from
3e7dd7b to
bffeb8e
Compare
jpnurmi
changed the title
Add copy-on-write (COW) semantics to sentry__value_clone() for list and object values. Instead of deep-copying items/pairs on clone, the new thing_t shares the underlying list_t/obj_t data via a data-level refcount. The actual copy is deferred to mutation time (thing_detach), making clone O(1) for flat containers. Two-level refcounting: - thing_t.refcount: how many sentry_value_t handles reference the thing - list_t.refcount / obj_t.refcount: how many thing_ts share the data Container children (nested lists/objects) are still recursively cloned to maintain value semantics at each nesting level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
In `thing_clone_children`, if `sentry__value_clone` fails for a nested container, the error was ignored. This resulted in clones that appeared usable but were missing nested data. Add a check to return false on clone failure, ensuring the entire operation fails rather than producing an incomplete result. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Keep object copy-on-write detach atomic when cloning keys fails. Track initialized pairs so partial clones can be freed without installing null keys. Co-Authored-By: OpenAI Codex <noreply@openai.com>
jpnurmi
force-pushed
the
jpnurmi/ref/copy-on-write
branch
from
bffeb8e to
837e73d
Compare
|
Linux numbers 0.040 ms 😎 |
limbonaut
left a comment
limbonaut
left a comment
There was a problem hiding this comment.
Superb 🤘This should be quite a noticeable performance improvement, I recon.
The deep clone trade-off could be minimized by using shared keys, which is worth consideration, imo.
| return NULL; | ||
| } | ||
| for (size_t i = 0; i < obj->len; i++) { | ||
| clone->pairs[i].k = sentry__string_clone(obj->pairs[i].k); |
There was a problem hiding this comment.
Potentially: Since keys are immutable, we could store them with a refcount and, instead of a copy here, increase the key's refcount. That would reduce allocations on mutate, especially in deep/wide objects where a single set_by_key could trigger N string copies. Just an idea to consider.
There was a problem hiding this comment.
Thanks for the proposal. 👍 Sounds like worth experimenting! Replacing plain C string keys with thing_t strings could achieve this at the cost of one additional allocation per key. 🤔
3 participants
Problem
Both out-of-process handlers, crashpad and native, flush the global scope to
__sentry-eventwhenever any scope properties change. This involves creating an event object and merging the current scope into it. The purpose of this is to make the event available for the separate handler/daemon process.In
sentry__scope_apply_to_event, the process of merging the scope into an event object is heavy. Passing references (event { tags: ref(scope->tags) }) is out of question, because that way anybefore_sendmodifications would flow back to the scope and cause data loss. Therefore, full clones (event { tags: clone(scope->tags) }) are passed instead. The problem is that the more data the scope has (tags, context, ...), the slower it gets to clone all that. Furthermore, in many cases, the clones are never modified, such as when flushing the scope for crashpad or native, or any native SDK user with a non-modifying or nobefore_sendhook.Solution
For container types,
sentry_value_talready stores the actual data separately inlist_tandobj_t. Instead of deep-copying the list items or object key-value pairs on clone, makelist_tandobj_trefcounted and share them between copies. Only if any clone modifies the data while refcount > 1, then the modified clone is detached from the shared data, and a deep copy is performed.Results
Close: #1551