Re. long vs. double, my opinion is: do not deal with long at all in the
API.
I doubt there could ever be a better performing long[]<->JsArrayNumber
conversion than iterating through the array and doing a long<->double
conversion for each value; so I wouldn't even provide utility methods
(as that would still somehow "encourage" the use of 'long' in client
code; and even if a 'long' never actually contain any value that
couldn't be stored in a 'double', it would still perform slower; and the
compiler is unlikely to ever know it and "tighten" the 'long' into a
'double' to optimize the generated code, even in future GWT versions)


http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/DataView.java
File user/src/com/google/gwt/typedarrays/client/DataView.java (right):

http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/DataView.java#newcode128
user/src/com/google/gwt/typedarrays/client/DataView.java:128: private
static native boolean isSupportedRuntime() /*-{
On 2012/01/06 14:01:03, Steffen Schäfer wrote:
On 2012/01/01 22:34:57, tbroyer wrote:
> Is there really a need for a distinct check besides
TypedArray.isSupported()?
> AFAICT, only Chrome m7 and m8 supported "typed arrays" without
supporting
> DataView, and those versions are no longer used on the web; or are
there
mobile
> browsers out there with similar "partial support"?

Yes, it's necessary. Mozilla is a little bit late with that
implementation:
https://bugzilla.mozilla.org/show_bug.cgi?id=575688

Oh, OK then.

At least for DataView. But you had specific isSupported() for all
typed-array types, is that needed at all?

http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/Int16Array.java
File user/src/com/google/gwt/typedarrays/client/Int16Array.java (right):

http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/Int16Array.java#newcode24
user/src/com/google/gwt/typedarrays/client/Int16Array.java:24: public
class Int16Array extends IntBasedTypedArray<Int16Array> {
On 2012/01/06 14:01:03, Steffen Schäfer wrote:
On 2012/01/01 22:34:57, tbroyer wrote:
> I'm not sure the IntBasedTypedArray intermediate type is worth it,
at least
for
> reading (the TypedArrays spec defines how values are converted, so
you can
> technically put a 'double' into an Int16Array; but for reading, I
think the
> getter return value should be of the type of the stored value: i.e.
'short'
for
> an Int16Array).

What do you think about that:
- adding setters for int, float, double and byte[], short[], int[],
float[],
double[], JsArrayInteger, JsArrayNumber to TypedArray. As you said,
the values
are automatically converted. so the user has all options to set
values.
- adding getters to the concrete Arrays using the matching Java types
for the
IntXX and FloatXX Arrays and Unit8->short, Uint16->int, Unit32->double
- Let's use that type mapping for the create methods too. If you have
an array
of a different type you can manually call the JsArrayUtil and use the
resulting
JsArray with the factory method.
=> Then we can remove the IntBasedTypedArray as you said without to
much
copy/paste code.

+1, except that I don't think an int+float+double overloads are
necessary: a double one would be enough, as any int or float can be cast
to a double. We'd have to check the generated code but I suspect 3
overloads with identical code might generate 3 identical JS functions
(i.e. not sure the compiler optimizations would "merge" them); I guess
the int->double and float->double conversions would be absolutely no-ops
in compiled code. This has to be weighed against a pure-Java
implementation though (maybe the int and float overloads would just call
the double overload in "client" code, so they would be optimized out, if
they happen to help in a "vm" implementation (to avoid re-casting back
to an int of float))

http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java
File user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java
(right):

http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java#newcode87
user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java:87: *
Converts a JsArrayString to a String[].
On 2012/01/06 14:01:03, Steffen Schäfer wrote:
On 2012/01/01 22:34:57, tbroyer wrote:
> Javadoc should make it *very* clear that it makes a copy in dev mode
but a
> "cast" in prod mode, so the returned array should only be used for
reading,
and
> the array passed as argument should also only be used for reading
while the
> returned array is in use: any change to one of the arrays would
induce
different
> behaviors in dev mode vs. prod mode.
>
> That means the method name ("unwrapArray") does not accurately
describes its
> behavior.
> In my own code, I call these "bridge" methods "forIteration" because
that's
> basically all they should be used for.

At the beginning, we only used these methods for dev mode and used a
if(GWT.isScript()) at all places where arrays are used. That's the
reason why
they have unsuitable names.
What do you think about using the names "asJsArray" and "asJavaArray"?
That's a
neutral description of what they do.
And yes, you are right, the JavaDoc should be clear about the side
effect.

"asXxx" APIs generally tend to return *views* over the same object (e.g.
ByteBuffer.asShortBuffer; Arrays.asList; etc.) so I'm not sure.
Because these are client-only methods, and the implementation once
compiled to JS is to actually return a "view" over the same data, then
maybe it'd be OK (with a big bold warning in the javadoc about the
different behavior in dev mode).

Maybe it should also be documented that you could produce strangely
behaving code due to GWT Compiler optimizations:
   JsArrayString sArr = ...;
   for (String s : asJavaArray(sArr)) {
      sArr.pop();
   }
That would compile to something like:
   var sArr = ...;
   for (var i=0, l=sArr.length; i < l; i++) {
      var s = sArr[i];
      sArr.pop();
   }
Notice how the array length is assumed to be a constant (which is the
case for a Java array, so that's what the GWT Compiler assumes) so the
loop is slightly optimized by looking it up once, rather than on each
iteration.
This is really an edge case though.

The rule of thumb is: do not ever modify a JsArray (resp. Java array)
while you're holding a reference to the "equivalent" Java array (resp.
JsArray).

http://gwt-code-reviews.appspot.com/1621803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to