Skip to content

[WIP] Update opt-in Copy-On-Write for filter items#154

Closed
Copilot wants to merge 1 commit intomainfrom
copilot/update-cow-filter-items
Closed

[WIP] Update opt-in Copy-On-Write for filter items#154
Copilot wants to merge 1 commit intomainfrom
copilot/update-cow-filter-items

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 9, 2026

  • Review existing codebase structure
  • Add useCopyOnWrite: boolean to DduOptions type in types.ts
  • Add useCopyOnWrite: false default to defaultDduOptions() in context.ts
  • Add cowifyItems function to utils.ts (Proxy-based COW with cloneCount tracking, structuredClone preference with JSON fallback warning)
  • Update #filterItems in ddu.ts to use cowifyItems when useCopyOnWrite: true and use state.items directly as originalItems
  • Update callFilters in ext.ts to accept optional getCloneCount and emit cloneCount in profile output
Original prompt

Update PR #153 (feat: introduce opt-in Copy-On-Write for filter items) with the following improvements and fixes:

Background:

  • The initial COW PR added an experimental useCopyOnWrite option and a Proxy-based cowifyItems implementation. Early tests showed COW was slower in practice due to extra deep clones used for originalItems and lack of instrumentation to see how often clones actually happened.

Goals for this update:

  1. Fix originalItems handling so COW gains effect

    • When useCopyOnWrite is true, avoid unnecessary structuredClone of originalItems prior to filtering.
    • Preserve correctness for preserveParentItems: originalItems must represent the unmodified items as produced by the source. When COW is enabled, state.items (the original array returned from gather) should be used directly as originalItems (no clone). Only the items passed to filters should be cowified proxies.
    • Ensure any code paths that relied on originalItems being a deep clone still work; add comments explaining the reason for using state.items when COW is enabled.
  2. Add instrumentation for COW clone occurrences

    • Modify cowifyItems to count how many per-item clones (ensureClone calls) happened during filter processing.
    • Expose that count to profiling output: when options.profile is true, include a field cloneCount in the profileResults summary for the filter run (or as an aggregated metric) so we can measure how often clones actually occur.
    • The count should be reset per filter invocation (or per callFilters invocation) so results are meaningful.
  3. Use structuredClone when available and avoid slow JSON fallback

    • In cowifyItems use globalThis.structuredClone if available (Deno) and only use JSON fallback as a last resort, but emit a one-time profile/warning via ddu#util#print_error if structuredClone is unavailable (so maintainers know fallback occurred). Prefer a graceful fallback but document limitations.
  4. Keep changes minimal, opt-in, and safe

    • Default behavior remains unchanged (useCopyOnWrite default false).
    • Add inline comments explaining tradeoffs.
    • Add tests/instructions in the PR description describing how to measure cloneCount and compare timings.

Files to modify in PR #153 branch:

  • denops/ddu/ddu.ts

    • In #filterItems: when useCopyOnWrite true, set items = cowifyItems(state.items) but set originalItems = state.items (no structuredClone). When useCopyOnWrite false, keep current structuredClone behavior.
    • Make sure preserveParentItems is fed originalItems representing source output (state.items), not the proxies.
  • denops/ddu/utils.ts

    • Update cowifyItems signature to return [proxiedItems, getCloneCount] or attach a symbol to return cloneCount.
    • Ensure ensureClone uses structuredClone if available. If structuredClone is missing, call a one-time denops.call('ddu#util#print_error', ...) to warn.
  • denops/ddu/ext.ts (callFilters)

    • When profile enabled, collect cloneCount from cowifyItems and add cloneCount field(s) to profileResults. If cowifyItems reports per-item clones, aggregate into a single number for the filter run.
  • denops/ddu/types.ts

    • No change to option name (useCopyOnWrite remains), but document option in defaultDduOptions and types comments.

Acceptance criteria / testing steps (include in PR description):

  • With useCopyOnWrite: false (default) behavior unchanged.
  • With useCopyOnWrite: true, run the same workload as before and gather ddu#util#get_logs('') output. The profile results should now include cloneCount. Compare runs to see if cloneCount is small (beneficial case) or large (not beneficial).
  • The heavy structuredClone of originalItems should be eliminated when useCopyOnWrite is true — measure memory/time reduction for large item sets.

Notes:

  • Keep the changes small and focused on the three items above. Provide a clear PR description explaining why originalItems clone removal is safe when COW is enabled (because proxies isolate mutations) and how to revert by disabling the option.

Please update PR branch for pull request #153 with these changes.

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot stopped work on behalf of Shougo due to an error March 9, 2026 11:45
@Shougo Shougo closed this Mar 10, 2026
@Shougo Shougo deleted the copilot/update-cow-filter-items branch March 10, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants