perf: zero-copy GET for large strings with CoW#7426
Open
romange wants to merge 1 commit into
Open
Conversation
Review Summary by QodoZero-copy GET for large strings with Copy-on-Write and streaming decode
WalkthroughsDescription• Implements zero-copy GET for large strings using Copy-on-Write mechanism - Borrowed views into CompactObj storage avoid std::string allocation - Supports NONE_ENC, ASCII1_ENC, ASCII2_ENC encodings - Chunked streaming decode for ASCII-packed values • Adds PendingRead pin registry with MPSC free list for safe concurrent access - Orphaning mechanism protects borrowed buffers during mutations - Drain cycle reclaims orphaned buffers on shard heartbeat • Extends reply builder with post-send pin release and streaming decode - WriteDecodedChunks streams decoded payload chunk-by-chunk into scratch - AddPostSendPin/SetPostSendUnpinFn manage pin lifetime across socket writes • Preserves zero-copy through MULTI/EXEC via CapturingReplyBuilder overrides - BulkStringView and BulkStringStreamed payload variants avoid materialization - Replay path streams directly to real sink without intermediate copy File Changes1. src/core/compact_object.cc
|
🤖 Augment PR SummarySummary: This PR wires an end-to-end zero-copy Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
romange
force-pushed
the
perf/zero-copy-core-primitives
branch
from
754d67b to
9e7b14e
Compare
Collaborator
Author
|
/config |
🛠️ Wiki configuration file settings:[github_app]
pr_commands = [
"/agentic_describe",
"/agentic_review"
]
[pr_compliance]
enabled = false
[pr_description]
enable_pr_diagram = false
[review_labels]
enabled = false
[review_agent]
enable_compliance_agent = false
🛠️ Local configuration file settings:
🛠️ Global configuration file settings:🛠️ PR-Agent final configurations:==================== CONFIG ====================
config.organization_id = 'codium.ai'
config.qodo_llm_gateway_metadata_tags = True
config.second_model_for_exhaustive_mode = 'openai/openai/o4-mini'
config.model = 'openai/openai/gpt-5.2'
config.model_turbo = 'openai/anthropic/claude-haiku-4-5'
config.model_reasoning = 'openai/vertex_ai/gemini-3.1-pro-preview'
config.fallback_models = ['openai/anthropic/claude-sonnet-4-6', 'openai/openai/gpt-5.2']
config.use_async_clone = True
config.git_provider = 'github'
config.publish_output = True
config.publish_output_no_suggestions = True
config.publish_output_progress = True
config.enable_v1_deprecation_banner = True
config.verbosity_level = 0
config.publish_logs = False
config.debug_mode = False
config.use_wiki_settings_file = True
config.use_repo_settings_file = True
config.use_global_settings_file = True
config.use_global_wiki_settings_file = False
config.disable_auto_feedback = False
config.ai_timeout = 300
config.response_language = 'en-US'
config.clone_repo_instead_of_fetch = True
config.always_clone = False
config.add_repo_metadata = True
config.add_repo_metadata_resolve_references = False
config.clone_repo_time_limit = 300
config.publish_inline_comments_fallback_batch_size = 5
config.publish_inline_comments_fallback_sleep_time = 2
config.max_model_tokens = 32000
config.custom_model_max_tokens = -1
config.patch_extension_skip_types = ['.md', '.txt']
config.extra_allowed_extensions = []
config.allow_dynamic_context = True
config.allow_forward_dynamic_context = True
config.max_extra_lines_before_dynamic_context = 12
config.patch_extra_lines_before = 5
config.patch_extra_lines_after = 1
config.ai_handler = 'litellm'
config.cli_mode = False
config.fetch_github_apps_from_platform = False
config.trial_git_org_max_invokes_per_month = 30
config.trial_ratio_close_to_limit = 0.8
config.quota_just_exceeded_message = '### Qodo reviews are paused for this user.\n\nTroubleshooting steps vary by plan [Learn more →](https://docs.qodo.ai/subscription-plans#what-you%E2%80%99ll-see-when-reviews-are-paused)\n\n\n**On a Teams plan?**\nReviews resume once this user has a paid seat *and* their Git account is linked in Qodo.\n[Link Git account →](https://docs.qodo.ai/subscription-plans#linking-a-git-account)\n\n**Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?**\nThese require an Enterprise plan - Contact us\n[Contact us →](https://docs.qodo.ai/qodo-support#support)\n'
config.invite_only_mode = False
config.enable_request_access_msg_on_new_pr = False
config.check_also_invites_field = False
config.allowed_users = []
config.calculate_context = True
config.disable_checkboxes = False
config.output_relevant_configurations = False
config.large_patch_policy = 'clip'
config.seed = -1
config.temperature = 0.2
config.allow_dynamic_context_ab_testing = False
config.choose_dynamic_context_ab_testing_ratio = 0.5
config.ignore_pr_title = ['^\\[Auto\\]', '^Auto']
config.ignore_pr_target_branches = []
config.ignore_pr_source_branches = []
config.ignore_pr_labels = []
config.ignore_ticket_labels = []
config.allow_only_specific_folders = []
config.ignore_pr_authors = 'REDACTED'
config.ignore_repositories = []
config.ignore_language_framework = []
config.is_auto_command = False
config.is_new_pr = False
config.enable_ai_metadata = True
config.present_reasoning = True
config.max_tickets = 10
config.max_tickets_chars = 8000
config.extract_tickets_from_pr_title = False
config.max_tickets_from_title = 1
config.title_extraction_skip_patterns = ['^revert\\b', '^merge\\b', '^release\\b', '^backport\\b', '^bump\\b']
config.prevent_any_approval = False
config.enable_comment_approval = False
config.enable_auto_approval = False
config.auto_approve_for_low_review_effort = -1
config.auto_approve_for_no_suggestions = False
config.ensure_ticket_compliance = False
config.new_diff_format = True
config.new_diff_format_add_external_references = True
config.tasks_queue_ttl_from_dequeue_in_seconds = 1200
config.enable_custom_labels = False
config.custom_labels_discovery_prefixes = ['pr_agent:', 'qodo:']
==================== PR_REVIEWER ====================
pr_reviewer.require_score_review = False
pr_reviewer.require_tests_review = True
pr_reviewer.require_estimate_effort_to_review = True
pr_reviewer.require_can_be_split_review = False
pr_reviewer.require_security_review = True
pr_reviewer.require_todo_scan = False
pr_reviewer.require_ticket_analysis_review = True
pr_reviewer.require_ticket_labels = False
pr_reviewer.require_no_ticket_labels = False
pr_reviewer.check_pr_additional_content = False
pr_reviewer.persistent_comment = True
pr_reviewer.extra_instructions = ''
pr_reviewer.final_update_message = True
pr_reviewer.enable_review_labels_security = True
pr_reviewer.enable_review_labels_effort = True
pr_reviewer.enable_help_text = False
==================== PR_COMPLIANCE ====================
pr_compliance.enabled = False
pr_compliance.enable_rules_platform = False
pr_compliance.rule_providers = []
pr_compliance.rule_providers_max_rules = 300
pr_compliance.enable_security_section = True
pr_compliance.enable_ticket_section = True
pr_compliance.enable_codebase_duplication_section = True
pr_compliance.enable_custom_compliance_section = True
pr_compliance.require_ticket_analysis_review = True
pr_compliance.allow_repo_pr_compliance = True
pr_compliance.enable_global_pr_compliance = True
pr_compliance.max_lines_allowed = 2000
pr_compliance.local_wiki_compliance_str = ''
pr_compliance.global_wiki_pr_compliance = ''
pr_compliance.local_repo_compliance_str = ''
pr_compliance.global_repo_pr_compliance_str = ''
pr_compliance.global_compliance_str = ''
pr_compliance.enable_generic_custom_compliance_checklist = True
pr_compliance.persist_generic_custom_compliance_checklist = False
pr_compliance.display_no_compliance_only = False
pr_compliance.enable_security_compliance = True
pr_compliance.enable_update_pr_compliance_checkbox = True
pr_compliance.enable_todo_scan = False
pr_compliance.enable_ticket_labels = False
pr_compliance.enable_no_ticket_labels = False
pr_compliance.check_pr_additional_content = False
pr_compliance.enable_compliance_labels_security = True
pr_compliance.enable_user_defined_compliance_labels = True
pr_compliance.enable_estimate_effort_to_review = True
pr_compliance.max_rag_components_to_analyze = 5
pr_compliance.min_component_size = 5
pr_compliance.persistent_comment = True
pr_compliance.enable_help_text = False
pr_compliance.extra_instructions = ''
==================== PR_DESCRIPTION ====================
pr_description.publish_labels = False
pr_description.add_original_user_description = True
pr_description.generate_ai_title = False
pr_description.extra_instructions = ''
pr_description.enable_pr_type = True
pr_description.final_update_message = True
pr_description.enable_help_text = False
pr_description.enable_help_comment = False
pr_description.bring_latest_tag = False
pr_description.enable_pr_diagram = False
pr_description.pr_diagram_direction = 'LR'
pr_description.publish_description_as_comment = False
pr_description.publish_description_as_comment_persistent = True
pr_description.enable_semantic_files_types = True
pr_description.collapsible_file_list = 'adaptive'
pr_description.collapsible_file_list_threshold = 8
pr_description.inline_file_summary = False
pr_description.use_description_markers = False
pr_description.include_generated_by_header = True
pr_description.enable_large_pr_handling = True
pr_description.max_ai_calls = 4
pr_description.auto_create_ticket = False
==================== PR_AGENTIC_DESCRIPTION ====================
pr_agentic_description.enable_file_changes_section = True
pr_agentic_description.file_listing_style = 'cards'
pr_agentic_description.publish_as_comment = True
==================== PR_QUESTIONS ====================
pr_questions.aware_ai_handler = False
pr_questions.enable_help_text = False
==================== PR_CODE_SUGGESTIONS ====================
pr_code_suggestions.suggestions_depth = 'exhaustive'
pr_code_suggestions.commitable_code_suggestions = False
pr_code_suggestions.decouple_hunks = False
pr_code_suggestions.dual_publishing_score_threshold = -1
pr_code_suggestions.focus_only_on_problems = True
pr_code_suggestions.allow_thumbs_up_down = False
pr_code_suggestions.enable_suggestion_type_reuse = False
pr_code_suggestions.enable_more_suggestions_checkbox = True
pr_code_suggestions.high_level_suggestions_enabled = True
pr_code_suggestions.extra_instructions = ''
pr_code_suggestions.enable_help_text = False
pr_code_suggestions.show_extra_context = False
pr_code_suggestions.persistent_comment = True
pr_code_suggestions.max_history_len = 5
pr_code_suggestions.apply_suggestions_checkbox = True
pr_code_suggestions.enable_chat_in_code_suggestions = True
pr_code_suggestions.apply_limit_scope = True
pr_code_suggestions.suggestions_score_threshold = 0
pr_code_suggestions.new_score_mechanism = True
pr_code_suggestions.new_score_mechanism_th_high = 9
pr_code_suggestions.new_score_mechanism_th_medium = 7
pr_code_suggestions.discard_unappliable_suggestions = False
pr_code_suggestions.num_code_suggestions_per_chunk = 3
pr_code_suggestions.num_best_practice_suggestions = 2
pr_code_suggestions.max_number_of_calls = 3
pr_code_suggestions.final_clip_factor = 0.8
pr_code_suggestions.demand_code_suggestions_self_review = False
pr_code_suggestions.code_suggestions_self_review_text = '**Author self-review**: I have reviewed the PR code suggestions, and addressed the relevant ones.'
pr_code_suggestions.approve_pr_on_self_review = False
pr_code_suggestions.fold_suggestions_on_self_review = True
pr_code_suggestions.publish_post_process_suggestion_impact = True
pr_code_suggestions.wiki_page_accepted_suggestions = True
pr_code_suggestions.enable_local_self_reflect_in_large_prs = False
pr_code_suggestions.simplify_response = True
==================== PR_CUSTOM_PROMPT ====================
pr_custom_prompt.prompt = 'The code suggestions should focus only on the following:\n- ...\n- ...\n...\n'
pr_custom_prompt.suggestions_score_threshold = 0
pr_custom_prompt.num_code_suggestions_per_chunk = 4
pr_custom_prompt.self_reflect_on_custom_suggestions = True
pr_custom_prompt.enable_help_text = False
==================== PR_ADD_DOCS ====================
pr_add_docs.extra_instructions = ''
pr_add_docs.docs_style = 'Sphinx'
pr_add_docs.file = ''
pr_add_docs.class_name = ''
==================== PR_UPDATE_CHANGELOG ====================
pr_update_changelog.push_changelog_changes = False
pr_update_changelog.extra_instructions = ''
pr_update_changelog.add_pr_link = True
pr_update_changelog.skip_ci_on_push = True
==================== PR_ANALYZE ====================
pr_analyze.enable_help_text = False
==================== PR_TEST ====================
pr_test.enable = True
pr_test.extra_instructions = ''
pr_test.testing_framework = ''
pr_test.num_tests = 3
pr_test.avoid_mocks = True
pr_test.file = ''
pr_test.class_name = ''
pr_test.enable_help_text = False
==================== PR_IMPROVE_COMPONENT ====================
pr_improve_component.num_code_suggestions = 4
pr_improve_component.extra_instructions = ''
pr_improve_component.file = ''
pr_improve_component.class_name = ''
==================== REVIEW_AGENT ====================
review_agent.enable_database_persistence = False
review_agent.llm_model = 'openai/openai/gpt-5.2_thinking'
review_agent.conversion_llm_model = 'openai/openai/gpt-5.2'
review_agent.enabled = True
review_agent.ensemble_models = ['openai/openai/gpt-5.2_thinking', 'openai/anthropic/claude-opus-4-6_thinking']
review_agent.publish_output = True
review_agent.enable_extended_mode = False
review_agent.enable_context_collector = False
review_agent.enable_issues_agent = True
review_agent.enable_ticket_context = True
review_agent.enable_compliance_agent = False
review_agent.enable_spec_agent = True
review_agent.enable_ui_agent = False
review_agent.enable_persona_agent = False
review_agent.persona_identifier = ''
review_agent.persona_repo_name = 'qodo-personas'
review_agent.persona_repo_default_branch = 'main'
review_agent.persona_auto_select = True
review_agent.persona_max_count = 2
review_agent.persona_source = 'repo'
review_agent.persona_url_target = 'repo'
review_agent.persona_portal_base_url = ''
review_agent.enable_deduplication = True
review_agent.enable_conversion_agent = True
review_agent.apply_conversion = True
review_agent.enable_precision_agent = False
review_agent.enable_cross_repo_agent = False
review_agent.enable_qodo_review_skill = False
review_agent.enable_past_bugs_collector = True
review_agent.enable_sandbox_usage = False
review_agent.sandbox_configs_repo_name = 'qodo-sandbox-configs'
review_agent.persistent_comment = True
review_agent.persistent_comment_notification = True
review_agent.enable_incremental_review = True
review_agent.enable_auto_fix = False
review_agent.enable_review_on_fix_pr = False
review_agent.rules_enabled = True
review_agent.requirements_gap_enabled = True
review_agent.llm_call_timeout = 180
review_agent.context_collector_llm_model = 'turbo'
review_agent.feedback_tool_llm_model = 'turbo'
review_agent.spec_llm_model = ''
review_agent.persona_llm_model = ''
review_agent.persona_selector_llm_model = ''
review_agent.conversion_batching_mode = 'batch'
review_agent.conversion_batch_size = 10
review_agent.cross_repo_llm_model = ''
review_agent.precision_llm_model = ''
review_agent.precision_max_llm_calls = 45
review_agent.precision_batching_mode = 'batch'
review_agent.precision_batch_size = 50
review_agent.precision_agent_vote_strategy = 'unanimous_discard'
review_agent.langsmith_project_name = 'review-agent'
review_agent.max_tokens_for_file = 'REDACTED'
review_agent.single_unified_diff_tokens_limit = 'REDACTED'
review_agent.max_llm_calls = 100
review_agent.max_llm_calls_limit = 100
review_agent.warn_when_remaining = 3
review_agent.context_collector_max_llm_calls = 6
review_agent.compliance_batch_size = 0
review_agent.past_bugs_max_results = 10
review_agent.past_bugs_llm_model = 'openai/gpt-5.4-mini'
review_agent.pass_previous_findings_to_agents = True
review_agent.deduplication_llm_max_tokens = 'REDACTED'
review_agent.publishing_action_level_rank_threshold = 0
review_agent.comments_location_policy = 'both'
review_agent.inline_comments_severity_threshold = 3
review_agent.prefer_single_line_comments = False
review_agent.issues_user_guidelines = ''
review_agent.compliance_user_guidelines = ''
review_agent.additional_repos = []
review_agent.demand_self_review = False
review_agent.self_review_text = '**Author self-review**: I have reviewed the code review findings, and addressed the relevant ones.'
review_agent.approve_pr_on_self_review = False
==================== PR_HELP ====================
pr_help.force_local_db = False
pr_help.num_retrieved_snippets = 5
==================== PR_NEW_ISSUE ====================
pr_new_issue.label_to_prompt_part = {'general': 'general question', 'feature': 'feature request (may already be addressed in the documentation)', 'bug': 'possible bug report (may be a by design behavior)'}
pr_new_issue.supported_repos = ['qodo-ai/pr-agent']
==================== PR_HELP_DOCS ====================
pr_help_docs.repo_url = ''
pr_help_docs.repo_default_branch = 'main'
pr_help_docs.docs_path = 'docs'
pr_help_docs.exclude_root_readme = False
pr_help_docs.supported_doc_exts = ['.md', '.mdx', '.rst']
pr_help_docs.enable_help_text = False
==================== PR_SIMILAR_ISSUE ====================
pr_similar_issue.skip_comments = False
pr_similar_issue.force_update_dataset = False
pr_similar_issue.max_issues_to_scan = 500
pr_similar_issue.vectordb = 'pinecone'
==================== PR_FIND_SIMILAR_COMPONENT ====================
pr_find_similar_component.class_name = ''
pr_find_similar_component.file = ''
pr_find_similar_component.search_from_org = False
pr_find_similar_component.allow_fallback_less_words = True
pr_find_similar_component.number_of_keywords = 5
pr_find_similar_component.number_of_results = 5
==================== BEST_PRACTICES ====================
best_practices.auto_best_practices_str = ''
best_practices.wiki_best_practices_str = ''
best_practices.global_wiki_best_practices = ''
best_practices.local_repo_best_practices_str = ''
best_practices.global_repo_best_practices_str = ''
best_practices.global_best_practices_str = ''
best_practices.organization_name = ''
best_practices.max_lines_allowed = 2000
best_practices.enable_global_best_practices = True
best_practices.allow_repo_best_practices = True
best_practices.enabled = True
==================== AUTO_BEST_PRACTICES ====================
auto_best_practices.enable_auto_best_practices = True
auto_best_practices.utilize_auto_best_practices = True
auto_best_practices.extra_instructions = ''
auto_best_practices.min_suggestions_to_auto_best_practices = 10
auto_best_practices.number_of_days_to_update = 30
auto_best_practices.max_patterns = 5
auto_best_practices.minimal_date_to_update = '2025-01-26'
auto_best_practices.generate_new_best_practices = False
==================== JIRA ====================
jira.jira_client_id = 'REDACTED'
jira.jira_app_secret = 'REDACTED'
==================== LINEAR ====================
linear.linear_client_id = 'REDACTED'
==================== PR_TO_TICKET ====================
pr_to_ticket.default_base_url = ''
pr_to_ticket.default_project_key = 'REDACTED'
pr_to_ticket.fallback_to_git_provider_issues = True
pr_to_ticket.direct_update_compliance = False
==================== PR_FIX ====================
pr_fix.enabled = False
==================== github_app ====================
github_app.bot_user = 'github-actions[bot]'
github_app.override_deployment_type = True
github_app.handle_pr_actions = ['opened', 'reopened', 'ready_for_review']
github_app.pr_commands = ['/agentic_describe', '/agentic_review']
github_app.feedback_on_draft_pr = False
github_app.handle_push_trigger = False
github_app.push_commands = ['/agentic_review']
github_app.ignore_pr_title = []
github_app.ignore_bot_pr = True |
romange
force-pushed
the
perf/zero-copy-core-primitives
branch
from
9e7b14e to
1557173
Compare
romange
force-pushed
the
perf/zero-copy-cow
branch
5 times, most recently
from
897be3d to
3eb5798
Compare
Collaborator
Author
|
/agentic_review |
Code Review by Qodo
1. Shutdown pin use-after-free
|
Comment on lines
+449
to
+455
| ~PinnedMap() { | ||
| // Thread shutdown: free any PendingRead entries that EngineShard::Heartbeat | ||
| // never had a chance to drain. Orphan buffers are best-effort — the | ||
| // thread's mimalloc heap is going away anyway. | ||
| for (auto& [_, pin] : pin_map_) | ||
| delete pin; | ||
| } |
There was a problem hiding this comment.
1. Shutdown pin use-after-free 🐞 Bug ☼ Reliability
PinnedMap::~PinnedMap deletes all PendingRead entries even though non-owning threads can still hold cmn::BorrowedString objects whose destructor calls ReleaseInternal() and dereferences the PendingRead* pin. This can crash during shard thread teardown / process shutdown if any borrowed GET reply is still in-flight when the shard thread’s TLS destructors run.
Agent Prompt
### Issue description
`PinnedMap::~PinnedMap()` currently `delete`s every `PendingRead*` in `pin_map_` during thread shutdown. However, `cmn::BorrowedString` can be destroyed on non-owning threads, and its destructor calls `BorrowedStringOps::ReleaseInternal()`, which dereferences the `PendingRead*` pin and decrements its atomic refcount. If the owning thread exits first and runs `~PinnedMap()`, later `BorrowedString` destruction will dereference freed memory (UAF) and can crash.
### Issue Context
The code explicitly documents that non-owning threads unpin by calling `BorrowedString::Unpin()` while the owning thread drains and deletes entries. Destructor-time deletion bypasses the refcount-based safety model.
### Fix Focus Areas
- src/core/compact_object.cc[449-455]
### Suggested fix
Change shutdown cleanup to **never delete pins that might still be referenced**:
- Option A (safest): remove the destructor cleanup entirely (intentionally leak `PendingRead` entries on thread shutdown; the thread heap/process is being torn down anyway).
- Option B: only delete entries whose `refcnt.load(acquire) == 0` (and free orphaned buffers for those), and **leave refcnt>0 entries allocated** to avoid UAF; optionally log/DFATAL in debug builds to catch unexpected outstanding borrows at shutdown.
Also consider (optional hardening): explicitly call `CompactObj::DrainPendingReads()` as part of shard shutdown ordering *before* TLS teardown, but this should be additive—not a substitute for making `~PinnedMap()` safe.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
PinnedMap being thread-local destructs during thread destruction.
Collaborator
Author
|
augment review |
| // facade::SinkReplyBuilder::ReplyScope contract. | ||
| StringResult ReadStringBorrow(DbIndex dbid, string_view key, const PrimeValue& pv, | ||
| EngineShard* es) { | ||
| static thread_local bool zero_copy_enabled = absl::GetFlag(FLAGS_get_zero_copy); |
There was a problem hiding this comment.
ReadStringBorrow caches FLAGS_get_zero_copy in a static thread_local, so runtime changes (e.g. via CONFIG SET if this ever becomes mutable) won’t propagate consistently across shard threads. Consider clarifying that get_zero_copy is startup-only, or otherwise ensuring updates are observed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| Send(iores.error().message()); | ||
| } | ||
|
|
||
| // Specialized overload for StringResult (3-variant: string / string_view / |
There was a problem hiding this comment.
This comment says StringResult is a 3-variant string / string_view / tiering future, but the variant is now std::string / cmn::BorrowedString / tiering future. Keeping this accurate matters because BorrowedString has pin/lifetime semantics that differ from a plain string_view.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
romange
force-pushed
the
perf/zero-copy-cow
branch
2 times, most recently
from
1c61903 to
f34ed26
Compare
Wire the zero-copy GET path end-to-end, building on the core primitives
(read_pending bit, TryGetRaw, pin registry) from the preceding PR.
`ReadStringBorrow` calls `pv.TryGetRaw()` instead of `pv.ToString()` for
large string values. `TryGetRaw` registers a read pin and stamps
`read_pending` as a side effect; the returned `RawBorrow.pin` is forwarded
to `SinkReplyBuilder::AddPostSendPin` so it is released once the socket
write completes.
`BorrowedString` carries `{encoded, pin, decoded_size, encoding}`:
- `NONE_ENC`: reply builder streams bytes directly via `SendBulkStringBorrowed`
(iovec reference, no copy)
- `ASCII1/ASCII2_ENC`: reply builder streams via `SendBulkStringStreamed`,
decoding the ASCII-packed source one chunk at a time into its scratch
buffer — no full decoded payload is ever materialised anywhere
`get_zero_copy` flag (default on) lets operators A/B benchmark the path
without a rebuild; cached thread-locally to avoid per-GET flag reads.
- `borrowed_string_views_total` (shard-side): incremented each time
`TryGetRaw` succeeds and a borrow is issued
- `borrowed_strings_sent_total` (coordinator-side): incremented each time
the borrowed path reaches the reply builder
- Both exposed in `INFO stats`
Collapses the zero-copy GET facade surface around a single move-only owning
type. cmn::BorrowedString carries encoded bytes + decoding metadata + an
opaque pin and its own release fn; the destructor releases the pin.
Capture/replay collapses two payload alternatives (BulkStringView +
BulkStringStreamed) into one unique_ptr<cmn::BorrowedString>; the variant
owns the pin until replay moves the borrow into the real sink.
dfly_facade now links dfly_core for detail::ascii_unpack (used by the
chunked decode helper inside reply_builder.cc).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
romange
force-pushed
the
perf/zero-copy-cow
branch
from
f34ed26 to
41e5378
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stacked on #7412 (
perf/zero-copy-core-primitives). Adds the reply-builderintegration, EngineShard wiring, and GET command changes on top of the
foundational primitives from that PR.
Summary
Wires the zero-copy GET path end-to-end for
LARGE_STR_TAGvalues withNONE_ENC,ASCII1_ENC, andASCII2_ENCencodings.GETon a largestring no longer allocates a
std::string; the reply builder streams bytesdirectly from the shard's
CompactObjstorage. Concurrent mutations aresafe via the Copy-on-Write mechanism from the base PR.
Changes
GET fast path (
string_family.cc)ReadStringBorrowcallspv.TryGetRaw()(which registers the read pin andstamps
read_pending) and forwardsRawBorrow.pinto the reply builder forpost-send release. Two sub-paths:
NONE_ENC:SendBulkStringBorrowed— iovec reference, zero bytes copiedASCII1/ASCII2_ENC:SendBulkStringStreamed— ASCII-packed source decodedchunk-by-chunk into the reply builder's scratch; no full decoded payload held anywhere
get_zero_copyflag (default on) for A/B benchmarking without rebuild.Reply builder (
facade/reply_builder.{h,cc})WriteDecodedChunks/SendBulkStringStreamed: stream-decode a packedsource into the scratch buffer with intermediate
Flush()for large valuesAddPostSendPin/SetPostSendUnpinFn: opaque pin released inSend()after the socket write completes
Capture/replay (
facade/reply_capture,reply_payload.h)CapturingReplyBuilderoverrides both send methods to record borrowedpayloads without materialising a
std::string. Replay forwards to the realsink's streaming path — zero-copy survives MULTI/EXEC squashing.
Wiring (
engine_shard.cc)InitShardThreadLocalStateinstalls the unpin function at startup.HeartbeatcallsCompactObj::DrainPendingReads().Stats (
server_state.h,engine_shard.h,server_family.cc)borrowed_string_views_total(shard) andborrowed_strings_sent_total(coordinator) in
INFO stats.Tests
GetLargeRawBorrowed/GetLargeRawBorrowedSquashed— NONE_ENC round-trip, stat countersGetLargeAsciiBorrowedChunked/GetLargeAsciiBorrowedChunkedSquashed— ASCII paths including scratch-flush sizesPendingReadPinOrphanDrain/PendingReadDrainNonOrphaned— pin lifecycle viaTryGetRaw+ CoW