[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