Repository navigation
optimize v3 ownership checking - #27708
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cd5065419
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $if ownership ? { | ||
| w.ownership_fork_for_parallel_check(tc) | ||
| } |
There was a problem hiding this comment.
Cloning only the ownership state here is not enough once -d ownership is allowed to use the pthread checker: while checking function bodies, ownership_fn_value_name_from_expr() can call ownership_register_fn_literal_signature(), which mutates fn_ret_types/fn_param_types through register_fn_signature(). Those maps are still shared by the shallow-copied TypeChecker, so non-Windows builds with enough functions to enter the parallel path and function literals in checked bodies can race on shared map writes or leave nondeterministic/lost synthetic signatures; the worker needs private/merged signature maps or ownership should remain serial for that path.
Useful? React with 👍 / 👎.
ec6ffa6 to
bf0fdff
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef66989a7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if tc.pending_ierror_errors.len > 0 { | ||
| tc.collect_selected_file_called_fns() | ||
| } | ||
| tc.filter_pending_ierror_errors() |
There was a problem hiding this comment.
When diagnostic_files is set and any invalid-IError-return candidates are pending, filter_pending_ierror_errors() appends the selected candidates to tc.errors after run_parallel_check() has already sorted and deduped the worker errors. In parallel chunks (and even the serial fallback through this path), those appended diagnostics can be left out of source order or duplicated; the previous flow sorted again after filtering, so this path should do the same after line 82 when pending errors may have been added.
Useful? React with 👍 / 👎.
Summary
Why
Compiling
ripgrep_vwith-prod -ownershipspent around 2.9s in the V3 checker. The main cost was ownership collection/pre-scan plus serialized semantic checking. This change keeps ownership diagnostics intact while allowing the checker to use the existing parallel function-body scheduler.Validation
./v -g -keepc -o ./vnew cmd/v./vnew fmt -w vlib/v3/types/checker.v vlib/v3/types/checker_ownership_d_ownership.v vlib/v3/types/checker_parallel.v./vnew -silent test vlib/v3/tests/review_ownership_pr_regressions_test.varr.first()movingarr[0]./vnew -prod -ownership -o /tmp/rg_v_prod_opt /Users/alexander/code/ripgrep_v/main.vreachedcheck (parallel) 996.13 mson the final codeBroader V3 suites were not clean in this worktree before this PR:
vlib/v3/tests/type_checker_errors_test.vstill hasbad_multi_return_match_tail_returncompiling successfully even with-no-parallel, and a heavy veb ownership case was killed by the OS during earlier validation.