Skip to content

feat(scala): support native copy for Scala collections - #4197

Open
dajiaohuang wants to merge 2 commits into
apache:mainfrom
dajiaohuang:feat/scala-native-copy-support
Open

dajiaohuang wants to merge 2 commits into
apache:mainfrom
dajiaohuang:feat/scala-native-copy-support

Conversation

@dajiaohuang

Copy link
Copy Markdown

Why?

Fory.copy did not provide native copy support for Scala immutable collections, mutable collections/maps, and ranges. This adds support while preserving object identity across mixed immutable/mutable cycles.

What does this PR do?

  • Adds native copy implementations for Scala collections, maps, and ranges.
  • Reuses a canonical copy registered during recursive copying, so cyclic graphs do not split one original object into multiple copies.
  • Adds five focused regression tests, including immutable List ↔ mutable map and immutable map ↔ mutable ListBuffer identity cycles.

The contributor's human-authored rationale, retained verbatim: “会产生副本不同步” (copies may become unsynchronized) and “是否完全做到区分单一副本和原对象” (whether the implementation fully distinguishes the single copy from the original object).

Related issues

Related to #4103.

Verification

The contributor personally ran and reviewed the focused NativeCopyTest checks on 2026-10-27:

  • In sbt: ++2.13.18 "fory-scala/testOnly org.apache.fory.serializer.scala.NativeCopyTest" — 5 tests run, 5 passed, 0 failed; exit code 0.
  • In sbt: ++3.3.8 "fory-scala/testOnly org.apache.fory.serializer.scala.NativeCopyTest" — 5 tests run, 5 passed, 0 failed; exit code 0.
  • git diff --check passed.

The two targeted runs cover the changed behavior. No full Scala module suite, CI run, or benchmark is claimed. This is correctness support for previously unsupported native Fory.copy, not a comparative performance claim; there is no equivalent native-copy baseline.

AI Contribution Checklist

  • Substantial AI assistance was used: yes.
  • This description includes the standardized AI Usage Disclosure below.
  • The contributor reviewed the exact candidate line by line, supplied the stated design/risk rationale, and can explain the implementation.
  • Line-by-line self-review was completed before the two fresh AI reviews.
  • Both fresh reviews used the required Fory-guided and independent-general workflows on the exact candidate.
  • No actionable review comments remained; both final outcomes and limitations are included below.
  • Final clean review results are embedded below as persisted evidence.
  • The contributor personally ran the focused checks and reviewed their results; exact commands and outcomes are recorded below.
  • Regression tests were added for the changed behavior.
  • No protocol/wire-format change or performance claim is made; no benchmark is represented as run.
  • The changed material's source trace and Apache-2.0 compatibility were checked and are disclosed below.

AI Usage Disclosure

  • substantial_ai_assistance: yes
  • scope: design drafting, Scala implementation, regression tests, and review assistance.
  • affected_files_or_subsystems: Scala collection, map, and range serializers and NativeCopyTest.
  • ai_review: The contributor reports line-by-line self-review and review of the exact candidate. Two fresh clean-context reviews were run on base HEAD 8e936331b6fa5c4b28ba356a10a518ea72b20ba4 and the candidate fingerprint SHA-256 2731b7a72ca0dcaa0567006ed6534b255162626a313128951941afcbb8575a79. Both returned no further actionable comments. Their final results and limitations are embedded below. Neither reviewer ran tests, builds, or benchmarks.
  • ai_review_artifacts: Final reviewer results are embedded here, identifying each workflow, reviewed base and candidate fingerprint, and its final result.

Final Fory-guided review — fory-fresh-guided-review-20261027-r2

Final result: No further actionable comments. The reviewer verified base HEAD 8e936331b6fa5c4b28ba356a10a518ea72b20ba4 and the expected changed-file set (the three serializers and NativeCopyTest.scala). The revised builder paths reuse the recursively registered canonical copy, addressing the mixed-cycle identity split; the tests cover both immutable/mutable cycle directions, mutable self-cycles, and indexed-sequence copying. git diff --check reported no issues. Limitation: this reviewer did not independently recompute the combined candidate hash and did not run tests, builds, lint, or benchmarks.

Final independent general review — fory-fresh-independent-review-20261027-r2

Final result: No further actionable comments. The reviewer verified the frozen candidate artifact SHA-256 2731b7a72ca0dcaa0567006ed6534b255162626a313128951941afcbb8575a79 and reviewed the stated base HEAD and changed-file set. It found the canonical-copy reuse addresses the cycle identity split and found no further actionable correctness/regression issue; git diff --check HEAD passed. Limitation: the independent worktree reconstruction did not match the frozen artifact, so this reviewer does not independently establish that the artifact bytes exactly correspond to the current worktree. It did not run tests, builds, or benchmarks.

After the reviews, S regenerated the review artifact from the current worktree and reported that it matched the same candidate fingerprint. This is S's reconciliation, not a claim that either reviewer independently reproduced exact worktree/hash correspondence.

  • human_verification: The contributor personally ran and reviewed the two focused Scala-version checks listed above; each ran 5 tests and passed 5/5 with exit code 0.
  • performance_verification: No benchmark or comparative performance result is claimed. This change enables a previously unsupported correctness operation; no equivalent native-copy baseline is available.
  • provenance_license_confirmation: The changed implementation follows existing Apache Fory source/API patterns and Scala standard collection APIs. The contributor's source trace found no copied third-party snippet, no incompatible third-party material, and no new dependency; the changes are Apache-2.0-compatible.

User-facing and compatibility impact

  • This adds behavior to the existing Fory.copy API; it does not add a public API signature.
  • No binary protocol or wire-format change is intended.

This branch has not been deployed

No deployments
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.

1 participant