[MLton] Apparent fix to PackReal

Matthew Fluet fluet at tti-c.org
Tue Feb 20 12:41:19 PST 2007


Eric McCorkle wrote:
> PackReal.toBytes was returning a reference to a single globally defined 
> array.

Yes, that looks like a bug.  It would probably be worthwhile to rename 
Vector.fromArray to Vector.fromArrayUnsafe to remind us that the 
operation doesn't produce an immutable copy of the array.  (In contrast, 
Array.vector (from the Basis Library) does make a copy.)

 > I changed pack-real.sml from:
> 
> 
> local
>    val a = ArrayUninit bytesPerElem
> in
>    fun toBytes (r: real): Word8Vector.vector =
>       (updA (a, 0, r)
>        ; Word8Vector.fromPoly (Vector.fromArray a))
> end
> 
> to:
> 
> 
> fun toBytes (r: real): Word8Vector.vector =
>    let
>       val a = Array.arrayUninit bytesPerElem
>    in
>      updA (a, 0, r)
>      ; Word8Vector.fromPoly (Vector.fromArray a)
>    end
> 
> 
> and it seems to work fine.  Someone please confirm that this is the 
> Right Thing™.

That's probably the best solution.

Lifting the Array.arrayUninit out of the function could mean less 
allocation per call to toBytes.  The lifted version isn't thread safe, 
so one should really use the One structure.

local
   val one = One.make (fn () => Array.arrayUninit bytesPerElem)
in
   fun toBytes (r: real): Word8Vector.vector =
     One.use (one, fn a =>
              (updA (a, 0, r)
               ; Word8Vector.fromPoly (Array.vector a)))
end

However, this still allocates a vector (as part of the Array.vector 
call); furthermore, Array.vector will copy Word8.word elements one at a 
time from the array to the resulting vector.

So, I think your solution is the fastest.  I'll check it in soon.




More information about the MLton mailing list