[racket-dev] [plt] Push #25821: master branch updated

In the test, if you get unlucky and there is a gc between the
definition of bx and unboxing of it then this test will pass in
correctly, I think (unless build-array complains if it gets #f?)

On Sun, Dec 2, 2012 at 11:44 PM,  <ntoronto at racket-lang.org> wrote:
> ntoronto has updated `master' from 325600b0cf to 8f17913d55.
>   http://git.racket-lang.org/plt/325600b0cf..8f17913d55
>
> =====[ One Commit ]=====================================================
> Directory summary:
>   74.0% collects/math/private/array/
>   25.9% collects/math/tests/
>
> ~~~~~~~~~~
>
> 8f17913 Neil Toronto <ntoronto at racket-lang.org> 2012-12-02 19:02
> :
> | Fixed memory leak in making arrays strict: doing so wouldn't clear
> | the reference to the original procedure, which itself could hold on
> | to a lot of memory
> :
>   A collects/math/tests/strictness-memory-leak-test.rkt
>   M .../math/private/array/typed-array-struct.rkt     | 28 ++++++++++++--------
>
> =====[ Overall Diff ]===================================================
>
> collects/math/private/array/typed-array-struct.rkt
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> --- OLD/collects/math/private/array/typed-array-struct.rkt
> +++ NEW/collects/math/private/array/typed-array-struct.rkt
> @@ -76,17 +76,23 @@
>
>  (: unsafe-build-array (All (A) (Indexes (Indexes -> A) -> (Array A))))
>  (define (unsafe-build-array ds f)
> -  (define size (check-array-shape-size 'unsafe-build-array ds))
> -  (define: data : (U #f (Vectorof A)) #f)
> -  (define (strict!)
> -    (set! data (inline-build-array-data ds (λ (js j) (f js)) A)))
> -  (define unsafe-proc
> -    (λ: ([js : Indexes])
> -      (let ([data data])
> -        (if data
> -            (unsafe-vector-ref data (unsafe-array-index->value-index ds js))
> -            (f js)))))
> -  (Array ds size ((inst box Boolean) #f) strict! unsafe-proc))
> +  ;; This box's contents get replaced when the array we're constructing is made strict, so that
> +  ;; the array stops referencing f. If we didn't do this, long chains of array computations would
> +  ;; keep hold of references to all the intermediate procs, which is a memory leak.
> +  (let ([f  (box f)])
> +    (define size (check-array-shape-size 'unsafe-build-array ds))
> +    ;; Sharp readers might notice that strict! doesn't check to see whether the array is already
> +    ;; strict; that's okay - array-strict! does it instead, which makes the "once strict, always
> +    ;; strict" invariant easier to ensure in subtypes, which we don't always have control over
> +    (define (strict!)
> +      (let* ([old-f  (unbox f)]
> +             [vs     (inline-build-array-data ds (λ (js j) (old-f js)) A)])
> +        ;; Make a new f that just indexes into vs
> +        (set-box! f (λ: ([js : Indexes])
> +                      (unsafe-vector-ref vs (unsafe-array-index->value-index ds js))))))
> +    (define unsafe-proc
> +      (λ: ([js : Indexes]) ((unbox f) js)))
> +    (Array ds size ((inst box Boolean) #f) strict! unsafe-proc)))
>
>  (: unsafe-build-strict-array (All (A) (Indexes (Indexes -> A) -> (Array A))))
>  (define (unsafe-build-strict-array ds f)
>
> collects/math/tests/strictness-memory-leak-test.rkt
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> --- /dev/null
> +++ NEW/collects/math/tests/strictness-memory-leak-test.rkt
> @@ -0,0 +1,16 @@
> +#lang racket
> +
> +(require math/array
> +         rackunit)
> +
> +;; Make a procedure that returns a random value to keep the optimizer from converting it to a
> +;; top-level, non-closure; if that happens, the module keeps a reference to it, which makes this
> +;; test always fail
> +(define bx (make-weak-box (let ([v  (random)]) (λ (js) v))))
> +
> +(define arr (build-array #() (weak-box-value bx)))
> +
> +;; Making `arr' strict should release the only remaining reference to the contents of `bx'
> +(array-strict! arr)
> +(collect-garbage)
> +(check-false (weak-box-value bx))