Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancements to typed arrays #39

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

Conversation

achirkin
Copy link
Contributor

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:

  • Naming of arrays/ArrayBuffer/DataView and their operations is mostly preserved; most of existing functions preserved, or placed into typeclass operations.
  • The main point is using actual numeric data types in typed arrays instead of parameterizing it over the JS array types. I believe this make use of the typed arrays more natural and flexible, while correspondence between JS types and Haskell is still obvious.
  • I separated IO and ST operations from pure operations, such that JavaScript.TypedArray.IO and JavaScript.TypedArray.ST are exactly the same, and any of these can be used together with JavaScript.TypedArray equally. I think it is not a strong deviation from standard way of structuring Haskell modules, right?
  • Added a number of very useful functions like fromList :: [a] -> TypedArray a and setList :: Int -> [a] -> IOTypedArray a -> IO ()
  • Common thaw/freeze and conversions to ByteArray . 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 in JavaScript.TypedArray.

John Lenz and others added 24 commits August 31, 2015 00:19
- the hs$export function had parameters fp2a fp2b but attempted to
  read fp1c and fp1d, which did not exist.
- Export.hs assumed deref was called hs$derefExportedValue but the
  actual function is hs$derefValue.
- A call to hs$retain was missing from the export.
xhr must be opened before any modification, also add required exports
add h$isBoolean to jsbits. Req'd by GHCJS.Foreign
requestAnimationFrame provides its callback with a DOMHighResTimeStamp,
measuring a monotonic clock time for each frame. I've extended the
bindings such that waitForAnimationFrame and inAnimationFrame provide
this value to the caller.
Add Performance.now and add time stamp information to requestAnimationFrame bindings
Seems like someone forgot to re-export Float32Array and  Float64Array in JavaScript.TypedArray from JavaScript.TypedArray.Internal.Types.
Also I would like to have (TypedArray a => JSRef -> a) interface for my WebGL things: some of the webGL methods return typed arrays of arbitrary type - https://developer.mozilla.org/en-US/docs/Web/API/ArrayBufferView
Added Float32/64 Typed array types
@luite
Copy link
Member

luite commented Oct 19, 2015

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:

  • I prefer explicit export lists for "general consumption" modules by the way, even if it adds some code bloat, since it helps readability and haddock. The same for the use of the preprocessor, so I've been really conservative with that, as you probably noticed.
  • Typeclasses do come with a cost if the instance cannot be inlined. Typed array code is typically performance sensitive, so I'd like to review the use of those everywhere, both in my version and your pr.
  • I don't agree with the 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.
  • I added 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.

@luite
Copy link
Member

luite commented Oct 19, 2015

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.

@achirkin
Copy link
Contributor Author

First, on comments:

  • Sorry for export lists, I can fix that if we decide to merge. As for CPP - I am afraid that in case of my implementation it will be quite a lot of code. And it is a bit easier to modify code - especially in Internal module with all js imports - it is anyway not exposed and not supposed to be visible in haddock. Also it's not clear to me: how usage of preprocessor affects haddock?
  • Typeclasses: in the original version TypedArray class is used anyway. I added extra classes for operations like 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: it certainly was stupid to use 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).
  • My main intention for 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:

  • In general I keep existing things the same, just adding functionality.
  • Several examples are here: https://github.com/achirkin/ghcjs-webgl/blob/master/test/SimpleWebGL.hs . Note (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.
  • In my version of typed arrays 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 :)
  • Another (very important for me as a ghcjs user) thing is in https://github.com/achirkin/fastvec . This is a vector library I develop for my own needs. Right now, with the new version of TypedArrays I can write all necessary instances for SomeTypedArray m (Vector n t) and use it in a webgl app (working on this today).

@stepcut
Copy link

stepcut commented Apr 14, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.