Open achirkin opened 8 years ago
Thanks for this PR! I wanted to go over the typed array bindings once more before releasing improved-base, and this should be a good way to get the discussion started. It's rather big, so it may take me some time to digest it all.
Can you give an example of how your code is more natural and flexible to use? Code that you couldn't have written before, or that can be written in a more general way now perhaps? In the end usability is what matters.
Some general comments:
Show
instances, and the same for the PToJSRef
/ PFromJSRef
instance pair, since these are all somewhat risky operations, allowing you to write unsafeThaw
or unsafeFreeze
without mentioning anything unsafe.h$fromListPrim
looks wrong by the way, it can't support updated thunks and 64 bit word and int types.GHCJS.Buffer
as a low-level mechanism to convert between JS typed arrays and GHCJS' internal structures indeed, and to manipulate the data structure directly, accessing components for free, without any new object allocation. As such it's probably less stable in API if we decide to switch to a different ByteArray representation at some point. So I think it's probably good to keep it around for a while.If you have time, hop on to #ghcjs
on freenode for more discussion by the way. I'm not there all day (and I'm having a short break in Scotland later this week, so I'll be offline a bit more), but I do try to check in a few times each day.
First, on comments:
thaw/freeze
, which are not performance critical, since typically invoked only once in a while. I am not sure in which cases the instances I wrote cannot be inlined - here I will definitely need your consultation. I added INLINE
pragma everywhere though, and could also try SPECIALIZE INSTANCE
at the places where instances contain type variables.show
on RHS; I can change it to unpack'
. Or did you mean something else? As for pToJSVal/pFromJSVal
- honestly, I did not think of consequences of that, I will remove it then (I have not used them yet anywhere).h$fromListPrim
is to convert number types that occur in JS TypedArrays (no 64 bit integers there). I copied your code from [JSVal] -> JSVal
, just removed unwrapping of values inside a list. So I also added seqList
to make the values fully evaluated and used this js function only in fromList
and setList
. Should there be the case when it does not work?GHCJS.Buffer
- ok, sound reasonable.As for examples of usage:
(28)uniformMatrix4fv
or (106)packVertColors
: I did not have to put any of type annotations, whereas in the original version Elem a
family is not injective. I also do not need to have any conversions when using e.g. Int32
and Int
as these are just different Haskell types: TypedArray Int32
and TypedArray Int
.SomeTypedArray m a
is exported too. I use it in https://github.com/achirkin/ghcjs-webgl/blob/master/src/GHCJS/WebGL/Raw.hs , because webgl does not care if array is mutable or not when reading data from it :)SomeTypedArray m (Vector n t)
and use it in a webgl app (working on this today).As an example of code that can't be written. I can find no way to produce a Uint8ClampedArray
value. I can produce a IOUint8ClampedArray
value -- but there is no way to freeze it. And the JavaScript.TypedArray.ST
module does not export the STTypedArray
class -- so I can't create anUint8ClampedArray
either.
I managed to merge my own typed arrays implementation into ghcjs-base one, trying to preserve your logic and structure. I do not really insist on merging this now, but rather want to invite you to discuss these changes.
Here are few things (that I remember now) about it:
IO
andST
operations from pure operations, such thatJavaScript.TypedArray.IO
andJavaScript.TypedArray.ST
are exactly the same, and any of these can be used together withJavaScript.TypedArray
equally. I think it is not a strong deviation from standard way of structuring Haskell modules, right?fromList :: [a] -> TypedArray a
andsetList :: Int -> [a] -> IOTypedArray a -> IO ()
thaw
/freeze
and conversions toByteArray
. This involves some trickery with type classes, but I hope this won't affect performance.Last thing: I think GHCJS.Buffer is a bit redundant. It looks like it is used only for conversions between JavaScript arrays/buffers and Haskell ByteArrays/Ptrs. Thus, maybe it is better to remove this module at all? Remaining conversion functions we can put into
GHCJS.Marshal
or somewhere else. In my version of typed arrays these conversions are done inJavaScript.TypedArray
.