Skip to content

typing: defer failed-application classification (perf)#1063

Merged
strub merged 1 commit into
mainfrom
fix-long-require
Jul 2, 2026
Merged

typing: defer failed-application classification (perf)#1063
strub merged 1 commit into
mainfrom
fix-long-require

Conversation

@strub

@strub strub commented Jul 1, 2026

Copy link
Copy Markdown
Member

Operator selection reported every rejected candidate through
EcUnify.classify_application, which peels the argument list one type at a
time, head-normalising via ty_hnorm (ty_subst (Tuni.subst (UniEnv.assubst ue)) _) at each step. UniEnv.assubst is subst_of_uf, materialising the
whole unifier by folding over UF.domain (every univar in the unienv), so
each candidate cost O(arity * |unienv|) instead of the O(arity)
incremental union-find of the previous single unification.

Overload resolution rejects candidates constantly on the success path
(any application of an overloaded name tries and drops the non-matching
instances), so this classification ran pervasively even though the
diagnostics it produces are only ever consumed when an application fails
outright and an error is raised. On a large formosa-keccak module this
turned a ~8s require into ~54s.

Defer the classification:

  • EcUnify: select_outcome's KO now carries a ... Lazy.t. select_op
    decides OK/KO with the cheap single unify top texpected and drops KO
    without ever forcing it; select_op_failures forces the thunks, for the
    error path only. classify_application is unchanged.

  • EcTyping.gen_select_op: the OK selection uses select_op; the failure
    list is a lazy closure forced only by tyerror_noop when building the
    UnappliedOp / UnknownVarOrOp error. select_pv likewise decides with a
    single unification, classifying the reason only in its failing branch.

Diagnostics are byte-identical (candidate order preserved). Keccak
require back to baseline; tests/op-application-errors.ec passes; unit
(84/84) and stdlib (128/128) green.

@strub strub force-pushed the fix-long-require branch 6 times, most recently from 04ac5ba to 54299ce Compare July 1, 2026 14:12
@strub strub self-assigned this Jul 1, 2026
Operator selection reported every rejected candidate through
EcUnify.classify_application, which peels the argument list one type at a
time, head-normalising via `ty_hnorm (ty_subst (Tuni.subst (UniEnv.assubst
ue)) _)` at each step. UniEnv.assubst is subst_of_uf, materialising the
whole unifier by folding over UF.domain (every univar in the unienv), so
each candidate cost O(arity * |unienv|) instead of the O(arity)
incremental union-find of the previous single unification.

Overload resolution rejects candidates constantly on the *success* path
(any application of an overloaded name tries and drops the non-matching
instances), so this classification ran pervasively even though the
diagnostics it produces are only ever consumed when an application fails
outright and an error is raised. On a large formosa-keccak module this
turned a ~8s require into ~54s.

Defer the classification:

- EcUnify: select_outcome's KO now carries a `... Lazy.t`. select_op
  decides OK/KO with the cheap single `unify top texpected` and drops KO
  without ever forcing it; select_op_failures forces the thunks, for the
  error path only. classify_application is unchanged.

- EcTyping.gen_select_op: the OK selection uses select_op; the failure
  list is a lazy closure forced only by tyerror_noop when building the
  UnappliedOp / UnknownVarOrOp error. select_pv likewise decides with a
  single unification, classifying the reason only in its failing branch.

Diagnostics are byte-identical (candidate order preserved). Keccak
require back to baseline; tests/op-application-errors.ec passes; unit
(84/84) and stdlib (128/128) green.
@strub strub force-pushed the fix-long-require branch from 54299ce to 5bd104d Compare July 2, 2026 14:51
@strub strub enabled auto-merge July 2, 2026 15:06
@strub strub added this pull request to the merge queue Jul 2, 2026
Merged via the queue into main with commit e1a1e84 Jul 2, 2026
19 checks passed
@strub strub deleted the fix-long-require branch July 2, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants