Live "click&drag" of 3d graphs runs at < 1 fps in recent versions of Racket

[jbclements]

I discovered recently that clicking and dragging of 3d graphs no longer works at a reasonable rate: specifically, clicking and dragging updates at a frame rate that's well below 1 frame per second on my machine. More like 1 frame per 5s. I've tried the example below on a series of older snapshots from Northwestern, and it looks like the 20170923-5e3a23886a and newer snapshots show the problem, but that 20170916-c68f42a1ef and older snapshots do not. Surprisingly, the 6.11 release does not show the performance regression, suggesting that something changed between september 16th & 23rd that didn't make it into the release.

I'm running Mac OS X 10.13.1 (High Sierra), and I've observed this problem on two different machines running this OS. I don't have another machine to test it on.

To see the problem, run the attached file (taken from the plot docs, with a frame% wrapper) from the command-line.

#lang racket

(require plot
         racket/gui)

(define f (new frame% [parent #f]
               [label "3D Graph"]
               [min-width 400]
               [min-height 400]))
(define ec (new editor-canvas% [parent f]))
(define t (new text%))
(send ec set-editor t)

(require (only-in plot/utils bounds->intervals linear-seq))
(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y)))))
(define x-ivls (bounds->intervals (linear-seq 2 8 16)))
(define y-ivls (bounds->intervals (linear-seq -5 5 16)))
(define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f))
(define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(send t insert
      (plot3d (rectangles3d (append*
                             (for/list ([y-ivl  (in-list y-ivls)]
                                        [y  (in-list y-mids)])
                               (for/list ([x-ivl  (in-list x-ivls)]
                                          [x  (in-list x-mids)])
                                 (define z (norm2 x y))
                                 (vector x-ivl y-ivl (ivl 0 z)))))
                            #:alpha 3/4
                            #:label "Appx. 2D Normal")))

(send f show #t)

[pnwamk]

It was this change that introduced the performance degradation you're noticing: https://github.com/racket/typed-racket/commit/9df037b0f6e6b2df3cc0993631bb2870367e291a

I double checked and indeed this change makes the performance normal again: https://github.com/pnwamk/typed-racket/commit/ec8cf19669438de570f4eea58257cfc30f50593a

This was a change that was not included in the 6.11 release after some debate, which is why you're not seeing it in the release but are seeing it on HEAD.

[jbclements]

On Nov 18, 2017, at 13:24, Andrew Kent notifications@github.com wrote:

It was this change that introduced the performance degradation you're noticing: racket/typed-racket@9df037b

I double checked and indeed this change makes the performance normal again: pnwamk/typed-racket@ec8cf19

This was a change that was not included in the 6.11 release after some debate, which is why you're not seeing it in the release but are seeing it on HEAD.

After re-reading the discussion associated with Sam’s and Ben’s discussion, it appears to me (IIUC) that 6.11 contained a temporary fix for “the zordoz problem”… what’s not clear to me is whether Ben is planning to fix this problem in the “right” way (whatever that means), and also whether that fix will also fix this problem.

Sounds like… I should open an issue in the racket/typed-racket repo?

John

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[pnwamk]

Sounds like… I should open an issue in the racket/typed-racket repo?

I would think so -- it seems like the underlying issue and its investigation is more likely to be typed-racket related than gui related at this point.

[samth]

Let's just use this issue so that we don't have to refer to the data gathered here in some other place as well.

On Nov 18, 2017 5:11 PM, "Andrew Kent" notifications@github.com wrote:

Sounds like… I should open an issue in the racket/typed-racket repo?

I would think so -- it seems like the underlying issue and its investigation is more likely to be typed-racket related than gui related at this point.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/racket/gui/issues/86#issuecomment-345475247, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO78xqQAWdTee-eNk8oqwuDQK4TEymAks5s31YQgaJpZM4QjIAu .

[jbclements]

Okay, sounds good to me. Flagging @bennn on this just in case he doesn't usually watch issues on this repo.

[bennn]

6.11 did not contain a temporary fix for "the zordoz problem"

The change in https://github.com/racket/typed-racket/commit/9df037b0f6e6b2df3cc0993631bb2870367e291a is probably slowing things down here because its making TR generate a contract that it didn't use to generate (unsafely).

An easy fix is to manually remove the slow contracts from plot. I'll look into that this week.

[samth]

We should make sure that removing those contracts doesn't potentially cause crashes before we actually take them out.

Also, if someone has this set up, a run of the contract profiler would be informative here.

[rfindler]

I'd be curious to see if there's something easy to fix in the contract system.

[bennn]

I don't understand the details yet, but the slowdown is definitely from a plot snip interacting with an untyped text% instance.

The snip comes from the plot3d function, whose return type is (U (Instance Snip%) Void).

Some examples:

If I change the example code to the the `Void` branch, dragging works:

``` #lang racket (require plot) (require (only-in plot/utils bounds->intervals linear-seq)) (define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y))))) (define x-ivls (bounds->intervals (linear-seq 2 8 16))) (define y-ivls (bounds->intervals (linear-seq -5 5 16))) (define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f)) (define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f)) (parameterize ((plot-new-window? #t)) (plot3d (rectangles3d (append* (for/list ([y-ivl (in-list y-ivls)] [y (in-list y-mids)]) (for/list ([x-ivl (in-list x-ivls)] [x (in-list x-mids)]) (define z (norm2 x y)) (vector x-ivl y-ivl (ivl 0 z))))) #:alpha 3/4 #:label "Appx. 2D Normal"))) ```

And if I change the example to use `plot3d-frame` directly, it's also fine (obviously):

``` #lang racket (require plot) (require (only-in plot/utils bounds->intervals linear-seq)) (define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y))))) (define x-ivls (bounds->intervals (linear-seq 2 8 16))) (define y-ivls (bounds->intervals (linear-seq -5 5 16))) (define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f)) (define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f)) (define s (plot3d-frame (rectangles3d (append* (for/list ([y-ivl (in-list y-ivls)] [y (in-list y-mids)]) (for/list ([x-ivl (in-list x-ivls)] [x (in-list x-mids)]) (define z (norm2 x y)) (vector x-ivl y-ivl (ivl 0 z))))) #:alpha 3/4 #:label "Appx. 2D Normal"))) (send s show #t) ```

Also, if I build the snip in Typed Racket and unsafely export it, then performance is ok:

``` #lang racket (require racket/gui) (module t typed/racket (require plot (only-in plot/utils bounds->intervals linear-seq)) (: norm2 (-> Real Real Real)) (define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y))))) (define x-ivls (bounds->intervals (linear-seq 2 8 16))) (define y-ivls (bounds->intervals (linear-seq -5 5 16))) (define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f)) (define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f)) (: make-stuff (-> ivl Real (Listof (Vectorof ivl)))) (define (make-stuff y-ivl y) (for/list : (Listof (Vectorof ivl)) ([x-ivl : ivl (in-list x-ivls)] [x : Real (in-list x-mids)]) (define z (norm2 x y)) (vector x-ivl y-ivl (ivl 0 z)))) (define stuff (for/list : (Listof (Listof (Vectorof ivl))) ([y-ivl : ivl (in-list y-ivls)] [y : Real (in-list y-mids)]) (make-stuff y-ivl y))) (define s (plot3d (rectangles3d (append* stuff) #:alpha 3/4 #:label "Appx. 2D Normal"))) (require typed/racket/unsafe) (unsafe-provide s)) (require 't) (define f (new frame% [parent #f] [label "3D Graph"] [min-width 400] [min-height 400])) (define ec (new editor-canvas% [parent f])) (define t (new text%)) (send ec set-editor t) (send t insert s) (send f show #t) ```

[rfindler]

I'm not understanding with "the Void branch" is. Can you elaborate a little bit for me?

[bennn]

Yep, calling plot3d either returns (void) or a plot3d snip. It returns (void) when the parameter plot-new-window? is #true --- that's what I meant by taking the void branch (were you able to see the code in my last comment?).

In other words:

• calling plot3d, getting a snip, and putting that in an untyped gui element is slow
• calling (parameterize ((plot-new-window? #true)) (plot3d ....)) and interacting wth the frame that call generates is OK

[rfindler]

Ah. Yes, I get it now. I guess the contract profiler can tell us which methods' contracts are the painful ones?

[bennn]

I guess, but I haven't had much luck using it.

I tried running the original code as raco contract-profile file.rkt and dragging the plot a little.

The profiler says only 2% of the running time was spent in contracts, and none of the contracts it lists seem to be snip methods

``` Running time is 2.17% contracts 138/6350 ms (->* ((recursive-contract (or/c (listof Treeof238) nonrender ... 16.5 ms (lib plot/private/gui/plot3d.rkt):19:9 plot3d 16.5 ms color%/c 21 ms /draw-lib/racket/draw.rkt:77:19 color% 21 ms (->* (path-string?) ((and/c path-string? relative-path?) (-> ... 10.5 ms /syntax/modcode.rkt:14:2 get-module-code 10.5 ms pdf-dc%/c 8.5 ms /draw-lib/racket/draw.rkt:86:19 pdf-dc% 8.5 ms (->* ((-> any/c any/c Real)) ((or/c Real #f) (or/c Real #f) ... 6 ms (lib plot/private/plot3d/surface.rkt):49:9 surface3d 6 ms (->* ((or/c natural? (sequence/c Real))) (Nonnegative-Real ( ... 6 ms (lib plot/private/plot2d/line.rkt):356:9 density 6 ms (-> (sequence/c any/c) (sequence/c any/c)) 8.5 ms (lib plot/private/common/utils.rkt):10:9 in-cycle* 8.5 ms (->* () #:rest (or/c (cons/c real? (listof pict-convertible? ... 7.5 ms /pict-lib/pict/main.rkt:108:3 vl-append 7.5 ms (-> (vectorof Index) Index (box/c boolean?) (-> #) (-> ... 12 ms (lib math/private/array/typed-array-struct.rkt):49:13 Array 12 ms (->* (exact-positive-integer? exact-positive-integer?) ((or/ ... 11 ms /draw-lib/racket/draw.rkt:49:11 make-monochrome-bitmap 11 ms (->* (exact-positive-integer? exact-positive-integer?) (any/ ... 11.5 ms /draw-lib/racket/draw.rkt:40:11 make-bitmap 11.5 ms (->* (sql-interval?) (any/c) any) 18.5 ms /db-lib/db/base.rkt:53:2 sql-interval->sql-time 18.5 ms ```

[rfindler]

Maybe there is some missing annotation somewhere? Does the regular profiler say anything useful? Is it time to do a binary search on the methods of snip%, hacking TR to (unsafely) remove the contracts?

[samth]

Rather than doing a binary search, could we just put some print outs in the relevant snip methods, and see what's being called a lot?

[jbclements]

Ping! I believe this hasn't yet been fixed, rendering plot zooming unusable. Is this going to go out as-is in the 6.12 release?

[bennn]

Right, this isn't fixed yet. Can you use either of these two work-arounds?

1. Don't create a frame, but set the plot-new-window? parameter

#lang racket

(require plot)

(require (only-in plot/utils bounds->intervals linear-seq))

(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y)))))
(define x-ivls (bounds->intervals (linear-seq 2 8 16)))
(define y-ivls (bounds->intervals (linear-seq -5 5 16)))
(define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f))
(define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(parameterize ((plot-new-window? #t))
  (plot3d (rectangles3d (append*
    (for/list ([y-ivl  (in-list y-ivls)]
               [y  (in-list y-mids)])
      (for/list ([x-ivl  (in-list x-ivls)]
                 [x  (in-list x-mids)])
        (define z (norm2 x y))
        (vector x-ivl y-ivl (ivl 0 z)))))
  #:alpha 3/4
  #:label "Appx. 2D Normal")))

2. Create a frame using plot3d-frame (i.e. a Typed Racket frame instead of a Racket frame)

#lang racket

(require plot)

(require (only-in plot/utils bounds->intervals linear-seq))

(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y)))))
(define x-ivls (bounds->intervals (linear-seq 2 8 16)))
(define y-ivls (bounds->intervals (linear-seq -5 5 16)))
(define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f))
(define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(define s
  (plot3d-frame (rectangles3d (append*
    (for/list ([y-ivl  (in-list y-ivls)]
               [y  (in-list y-mids)])
      (for/list ([x-ivl  (in-list x-ivls)]
                 [x  (in-list x-mids)])
        (define z (norm2 x y))
        (vector x-ivl y-ivl (ivl 0 z)))))
  #:alpha 3/4
  #:label "Appx. 2D Normal")))

(send s show #t)

[jbclements]

On Jan 9, 2018, at 11:49 PM, Ben Greenman notifications@github.com wrote:

Right, this isn't fixed yet. Can you use either of these two work-arounds?

I suppose I can, but … our users certainly won’t. Am I the only one in racket-land who zooms plots?

John

• Don't create a frame, but set the plot-new-window? parameter

lang racket

(require plot)

(require (only-in plot/utils bounds->intervals linear-seq))

(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y))))) (define x-ivls (bounds->intervals (linear-seq 2 8 16))) (define y-ivls (bounds->intervals (linear-seq -5 5 16))) (define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f)) (define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(parameterize ((plot-new-window? #t)) (plot3d (rectangles3d (append* (for/list ([y-ivl (in-list y-ivls)] [y (in-list y-mids)]) (for/list ([x-ivl (in-list x-ivls)] [x (in-list x-mids)]) (define z (norm2 x y)) (vector x-ivl y-ivl (ivl 0 z)))))

:alpha 3/4

:label "Appx. 2D Normal")))

• Create a frame using plot3d-frame (i.e. a Typed Racket frame instead of a Racket frame)

lang racket

(require plot)

(require (only-in plot/utils bounds->intervals linear-seq))

(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y))))) (define x-ivls (bounds->intervals (linear-seq 2 8 16))) (define y-ivls (bounds->intervals (linear-seq -5 5 16))) (define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f)) (define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(define s (plot3d-frame (rectangles3d (append* (for/list ([y-ivl (in-list y-ivls)] [y (in-list y-mids)]) (for/list ([x-ivl (in-list x-ivls)] [x (in-list x-mids)]) (define z (norm2 x y)) (vector x-ivl y-ivl (ivl 0 z)))))

:alpha 3/4

:label "Appx. 2D Normal")))

(send s show #t)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[samth]

I'm trying to look into this with either the regular profiler or the contract profiler, and I can't the regular profiler to not exit immediately. I've tried putting the computation in its own thread, and using the #:threads argument to profile, but nothing seems to work.

[samth]

This bad behavior also happens with the contract profiler, which produces output looking like what @bennn got.

[samth]

I would like to fix this for the 6.12 release, but so far I've been unable to get any useful output from either profiler at all. @rfindler do you use the profiler for DrRacket ever, and if so are there some tricks you could use?

Alternatively, we could re-patch 6.12 to not have the fix by @bennn for yet another release, but I'd really prefer to avoid that.

cc @racket/release

[bennn]

Another idea: remove the range part of the contract for the plot3d function.

[stamourv]

@samth: You're already late for the 6.12 release. The merge window is closed and testing has started.

[rfindler]

I think your suggestion of adding printfs to figure out what functions they are is probably easiest. We aren't looking for something subtle.

[samth]

@stamourv I'm not sure what conclusion to draw from your comment. Are you saying that this needs to get dealt with ASAP (which I knew)? Or that it's already too late and this regression will be in the release? Or that whatever fix we come up with will need release management approval? Or something else?

[rfindler]

He is reminding you of the timeline. Depending on where the bug is, you may or may not be too late for 6.12. In any case, I think waiting (more) is not wise.

[stamourv]

@rfindler is correct. A fix should have been in yesterday. That's what the merge window is for. Today testing has started. If you don't want this regression in 6.12, then back out the change ASAP before we get too far into testing.

This is the second release in a row (and I believe it has happened before) that last-minute fixes/back-outs in TR come in after the end of the merge window. This delays the release and leads to tedious email debates. I would very much prefer to avoid both of these.

So please, in the future either:

• Take care of these kinds of issues earlier in the release process. There's a heads up a week before the start of the merge window, precisely so you know it's coming.
• Have a more thorough vetting of changes to TR, to help avoid merging changes that have to be backed out.

[samth]

I've opened a very simple pull request to disable the contracts for these specific functions. I believe that this should be safe.

Unfortunately, adding printfs in the plot code didn't provide me any insight, since the rate of event handling seemed to be similar before and after this patch. As it is, it seems that the draw, on-motion, and on-event callbacks are prominent, but it's also possible that the code that's the issue is between the plot snips and the image-snip class, or somewhere else that I don't understand. It's also possible that the multiple layers of contracts here (between plot-snip and plot3d.rkt, and the to the client code @jbclements wrote) lead to multiple-wrapping that has major performance problems.

[jbclements]

Many many thanks to all of you for your last-minute work on this. I and other plot-zoomers are grateful!

[jbclements]

Ugh. 2d plots not fixed. I think this is the right place for this. Fix is probably the same.

[samth]

@jbclements can you test if a similar fix works there?

[jbclements]

can't now, back at probably 1:30 PST....

[bennn]

A similar fix works for me. Here's the program I used:

#lang racket

(require plot racket/gui)

(define f (new frame% [parent #f] [label "3D Graph"] [min-width 400] [min-height 400]))
(define ec (new editor-canvas% [parent f]))
(define t (new text%))
(send ec set-editor t)

(require (only-in plot/utils bounds->intervals linear-seq))
(define (norm2 x y) (exp (* -1/2 (+ (sqr (- x 5)) (sqr y)))))
(define x-ivls (bounds->intervals (linear-seq 2 8 16)))
(define y-ivls (bounds->intervals (linear-seq -5 5 16)))
(define x-mids (linear-seq 2 8 15 #:start? #f #:end? #f))
(define y-mids (linear-seq -5 5 15 #:start? #f #:end? #f))

(send t insert
(plot (function values) #:x-min 0 #:x-max 10 #:y-max 10))

(send f show #t)

[jbclements]

I can confirm that making a change analogous to

https://github.com/racket/plot/commit/65006c63385671cb05cd638c8e7a93e8e873f4f6

(that is: add require of typed/racket/unsafe, change 'provide' to 'unsafe-provide')

on the plot.rkt file solves the performance problem associated with 2d plots.

Looks like this may be subsumed by

https://github.com/racket/plot/pull/30

?

[stamourv]

racket/plot#30 is not a conservative change, so not a viable solution for 6.12. Whereas @bennn's change is.