racket / typed-racket

Typed Racket
Other
520 stars 103 forks source link

Should simple-result-> stop using `with-contract-continuation-mark` ? #1292

Open bennn opened 1 year ago

bennn commented 1 year ago

The simple-result-> contract should maybe stop using with-contract-continuation-mark (wccm) to run as quickly as possible.

Here is one program where wccm adds zero benefit but has big costs. This program goes through some (-> any real?) contracts many times:

#lang typed/racket

(module uuu racket
  (struct posn (x y))
  (provide (struct-out posn)))

(require/typed 'uuu
  (#:struct posn ((x : Real) (y : Real))))

(define origin (posn 0 0))

(for ((i (in-range (expt 10 8))))
  (posn-x origin)
  (posn-y origin))

It takes about 6 seconds to run.

With unsafe-require/typed it takes <1 second.

Ok, so there's a big cost due to contracts. But, running raco contract-profile main.rkt on this program says 0% of the runtime is due to contracts. Each contract check is too fast for the sampler to see. Increasing the sampling rate does not seem to help. Getting rid of the simple-result-> optimization does help: 17.62% contracts.

With simple-result-> and without the wccm, the program takes about 5 seconds.

To sum up:

So maybe the marks should be off by default and available on an opt-in basis.

rfindler commented 1 year ago

What you write makes a lot of sense, thanks!

Can you supply a version of this program that uses only the contract library (not TR) so I can more easily check the details?

Robby

On Tue, Nov 22, 2022 at 5:14 PM Ben Greenman @.***> wrote:

The simple-result-> contract should maybe stop using with-contract-continuation-mark (wccm) to run as quickly as possible.

Here is one program where wccm adds zero benefit but has big costs. This program goes through some (-> any real?) contracts many times:

lang typed/racket

(module uuu racket (struct posn (x y)) (provide (struct-out posn)))

(require/typed 'uuu (#:struct posn ((x : Real) (y : Real))))

(define origin (posn 0 0))

(for ((i (in-range (expt 10 8)))) (posn-x origin) (posn-y origin))

It takes about 6 seconds to run.

With unsafe-require/typed it takes <1 second.

Ok, so there's a big cost due to contracts. But, running raco contract-profile main.rkt on this program says 0% of the runtime is due to contracts. Each contract check is too fast for the sampler to see. Increasing the sampling rate does not seem to help. Getting rid of the simple-result-> optimization does help: 17.62% contracts.

With simple-result-> and without the wccm, the program takes about 5 seconds.

To sum up:

  • The contract profiler doesn't see the marks, but these marks cost about 1 second on this 6-second example.

So maybe the marks should be off by default and available on an opt-in basis.

— Reply to this email directly, view it on GitHub https://github.com/racket/typed-racket/issues/1292, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBNMA7IU4EQOEK6ADBLJDWJVHWDANCNFSM6AAAAAASILDZRM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

rfindler commented 1 year ago

Oh, I see now. I mistakenly thought simple-result-> was a helper function somewhere in the contract library. I see now that it is a contract combinator in TR.

Looking at the code, I don't mean to dispute the "importance in practice" claim written there, but I would guess that it is not uncommon for (equal? arity (procedure-arity v)) to be true and (equal? 1 (procedure-result-arity v))) to be false (because the compiler might have just given up on trying to figure out the result arity). And I think that would mean you can skip the make-keyword-procedure call, which might be significant?

bennn commented 1 year ago

Looking at the code, I don't mean to dispute the "importance in practice" claim written there, but I would guess that it is not uncommon for (equal? arity (procedure-arity v)) to be true and (equal? 1 (procedure-result-arity v))) to be false (because the compiler might have just given up on trying to figure out the result arity). And I think that would mean you can skip the make-keyword-procedure call, which might be significant?

That sounds likely to me.

It doesn't help for the program above (because the functions involved are struct accessors, and get the current fast path), but seems worth investigating.

I started a PR: https://github.com/racket/typed-racket/pull/1295