Avoid costly errors in JS witness generation #830
Conversation
src/base/constraint.ml
Outdated
| let of_basic = function T x -> x | _ -> failwith "different constructor" | ||
| let of_basic = function | ||
| | T x -> | ||
| x | ||
| | _ -> | ||
| raise (Failure "different constructor") |
There was a problem hiding this comment.
I'm just replacing failwith s with raise (Failure s). This makes a difference because in JS, we use a custom override of the failwith function, to have it throw JS errors instead of strings. This is very important in a couple of circumstances where the error implies an actual failure condition. Here, the error is just used for control flow and always caught a level earlier, so we don't need a pretty error.
There was a problem hiding this comment.
If you don't want to pay the cost of the exception here, can you change the code above it to use an val of_basic : ('v, 'f) basic -> ('v, 'f) t option instead.
src/base/constraint.ml
Outdated
| let of_basic = function T x -> x | _ -> failwith "different constructor" | ||
| let of_basic = function | ||
| | T x -> | ||
| x | ||
| | _ -> | ||
| raise (Failure "different constructor") |
There was a problem hiding this comment.
If you don't want to pay the cost of the exception here, can you change the code above it to use an val of_basic : ('v, 'f) basic -> ('v, 'f) t option instead.
src/base/request.ml
Outdated
| failwith | ||
| ( "Unhandled request: " | ||
| ^ Core_kernel.String.concat ~sep:"\n" label_stack ) | ||
| raise |
There was a problem hiding this comment.
As above, this should probably return an option, and the caller should handle the error by constructing an appropriate error message instead. This also has the benefit of never constructing this string if it isn't needed.
There was a problem hiding this comment.
good point! will change
src/base/checked_runner.ml
Outdated
| As_prover.Provider.run p (Run_state.stack s) (get_value s) | ||
| (Run_state.handler s) | ||
| As_prover.Provider.run p (get_value s) (Run_state.handler s) | ||
| |> Option.value_exn |
There was a problem hiding this comment.
If you do this in a match statement, then we'll only compute the error message when there's actually an error.
Instead of throwing JS errors, which are costly because they collect a full stack trace, this changes some core parts of snarky to throw strings instead. Throwing strings is very fast, and this change leads to a huge speedup during witrness generation, see o1-labs/o1js#1092