Add TypedArray C API #2165

galpeter · 2018-01-12T18:50:08Z

New API functions added:

jerry_value_is_typedarray
jerry_create_typedarray
jerry_create_typedarray_with_arraybuffer_sz
jerry_create_typedarray_with_arraybuffer
jerry_get_typedarray_type
jerry_get_typedarray_length
jerry_get_typedarray_buffer

rerobika · 2018-01-22T10:36:48Z

docs/02.API-REFERENCE.md

+**Summary**
+
+Create a jerry_value_t representing an TypedArray object using
+an already exisint ArrayBuffer object.


There is a typo in word existing.

rerobika · 2018-01-22T10:36:54Z

docs/02.API-REFERENCE.md

+**Summary**
+
+Create a jerry_value_t representing an TypedArray object using
+an already exisint ArrayBuffer object and by specifying the byteOffset, and length properties.


rerobika · 2018-01-22T10:37:03Z

jerry-core/api/jerry.c

+ * Notes:
+ *      * returns TypeError if an incorrect type (class_name) is specified.
+ *      * byteOffset property will be set to 0.
+ *      * byteLength property will be a muliple of the length parameter (based on the type).


There is a typo in word multiple.

galpeter · 2018-01-22T14:44:59Z

@rerobika Thanks for the checks! I've updated the PR.

zherczeg · 2018-01-23T09:52:41Z

docs/02.API-REFERENCE.md

+
+```c
+jerry_value_t
+jerry_create_typedarray_with_arraybuffer (jerry_typedarray_class_t class_name,


jerry_create_typedarray_for_arraybuffer ?

zherczeg · 2018-01-23T09:53:59Z

docs/02.API-REFERENCE.md

+
+  jerry_typedarray_class_t klass = jerry_get_typedarray_type (typedarray);
+
+  // klass is now JERRY_TYPEDARRAY_UINT32


Yeah, I've used klass as class is a keyword in C++. So if someone copies the code into c++ it would still compile without problems.

zherczeg · 2018-01-23T09:56:22Z

docs/02.API-REFERENCE.md

+
+```c
+bool
+jerry_value_is_typedbuffer (const jerry_value_t value)


jerry_value_is_typedarray

zherczeg · 2018-01-23T09:57:01Z

docs/02.API-REFERENCE.md

+```
+
+- `class_name` - type of TypedArray to create
+- `length` - length of the new TypedArray


number of items?

zherczeg · 2018-01-23T09:59:23Z

docs/02.API-REFERENCE.md

+ - JERRY_TYPEDARRAY_INT32 - represents the Int32Array TypedArray
+ - JERRY_TYPEDARRAY_FLOAT32 - represents the Float32Array TypedArray
+ - JERRY_TYPEDARRAY_FLOAT64 - represents the Float64Array TypedArray
+ - JERRY_TYPEDARRAY_UNKNOWN - represents an invalid TypedArray


I would prefer INVALID over UNKNOWN. The latter says it is a typedarray, but its properties are unknown. This is not true.

zherczeg · 2018-01-23T10:00:30Z

docs/02.API-REFERENCE.md

+```c
+jerry_value_t jerry_get_typedarray_buffer (jerry_value_t value,
+                                           jerry_length_t *byteOffset,
+                                           jerry_length_t *byteLength);


Can these be NULL? If not, why?

yes, they can be null. Then the developer would not get the underlying array buffer information.

zherczeg · 2018-01-23T10:01:23Z

docs/02.API-REFERENCE.md

+
+- `value` - TypedArray to get the ArrayBuffer from
+- `byteOffset` - start offset of the ArrayBuffer for the TypedArray
+- `byteLength` - number of bytes used from the ArrayBuffer for the TypedArray


Why byteLength when we use items in other cases?

byteLength here means the length of the underlying arraybuffer

to be more precise the number of bytes used from the arraybuffer. That is why this is not the same as the item number/count.

galpeter · 2018-01-30T09:19:11Z

@zherczeg I've updated the PR and did a bit of renaming (typedarray_class -> typedarray_type)

zherczeg

LGTM after minor update

zherczeg · 2018-01-30T10:50:56Z

docs/02.API-REFERENCE.md

+
+- `value` - TypedArray to get the ArrayBuffer from
+- `byteOffset` - returns the start offset of the ArrayBuffer for the TypedArray
+- `byteLength` - returns the number of bytes used from the ArrayBuffer for the TypedArray


Please mention that these are optional arguments.

I've update the PR with the "optional" markings for the parameters.

zherczeg

Still LGTM.

LaszloLango · 2018-02-05T13:51:37Z

jerry-core/api/jerry.c

+
+#undef TYPEDARRAY_ENTRY
+};
+#endif


missing comment:
#endif /* !CONFIG_DISABLE_ES2015_TYPEDARRAY_BUILTIN */

LaszloLango · 2018-02-05T13:56:24Z

jerry-core/api/jerry.c

+                                            jerry_length_t byte_offset, /**< offset for the ArrayBuffer */
+                                            jerry_length_t length) /**< number of elements to use from ArrayBuffer */
+{
+#ifndef CONFIG_DISABLE_ES2015_TYPEDARRAY_BUILTIN


jerry_assert_api_available ();

LaszloLango · 2018-02-05T13:58:01Z

jerry-core/api/jerry.c

+ *         TypeError if the object is not a TypedArray.
+ */
+jerry_value_t
+jerry_get_typedarray_buffer (jerry_value_t value, jerry_length_t *byte_offset, jerry_length_t *byte_length)


Missing comments of function parameters

LaszloLango · 2018-02-05T14:00:49Z

tests/unit-core/test-typedarray.c

+#include <stdio.h>
+
+// TODO: remove define if the arraybuffer with external buffer support landed
+#define HAS_EXTERNAL_ARRAYBUFFER_API 0


#2162 is landed

LaszloLango · 2018-02-05T14:07:33Z

jerry-core/api/jerry.c

+  if (prototype_id == 0)
+  {
+    return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("incorrect type for TypedArray.")));
+  }


lines 3004-3022 is the same as 2949-2968, it would be great if we can merge them (eg.: use a helper)

galpeter · 2018-02-05T16:39:19Z

@LaszloLango Updated the PR based on the requests (and also rebased it)

New API functions added: - jerry_value_is_typedarray - jerry_create_typedarray - jerry_create_typedarray_for_arraybuffer_sz - jerry_create_typedarray_for_arraybuffer - jerry_get_typedarray_type - jerry_get_typedarray_length - jerry_get_typedarray_buffer JerryScript-DCO-1.0-Signed-off-by: Peter Gal [email protected]

LaszloLango

LGTM

galpeter added enhancement An improvement feature request Requested feature api Related to the public API ES2015 Related to ES2015 features labels Jan 12, 2018

galpeter mentioned this pull request Jan 12, 2018

C API for ArrayBuffer/TypedArray #1832

Closed

galpeter force-pushed the c-api-typedarray branch from 5348e04 to d104953 Compare January 15, 2018 11:51

rerobika reviewed Jan 22, 2018

View reviewed changes

galpeter force-pushed the c-api-typedarray branch from d104953 to cbe9917 Compare January 22, 2018 14:44

zherczeg requested changes Jan 23, 2018

View reviewed changes

galpeter force-pushed the c-api-typedarray branch 4 times, most recently from 69189b6 to 25d89d8 Compare January 29, 2018 16:25

zherczeg approved these changes Jan 30, 2018

View reviewed changes

galpeter force-pushed the c-api-typedarray branch from 25d89d8 to 31c8342 Compare January 30, 2018 10:55

zherczeg approved these changes Feb 5, 2018

View reviewed changes

LaszloLango requested changes Feb 5, 2018

View reviewed changes

galpeter force-pushed the c-api-typedarray branch from 31c8342 to 05c9ab1 Compare February 5, 2018 16:37

galpeter force-pushed the c-api-typedarray branch from 05c9ab1 to 638795c Compare February 5, 2018 17:22

LaszloLango approved these changes Feb 6, 2018

View reviewed changes

LaszloLango merged commit 8041953 into jerryscript-project:master Feb 6, 2018

zherczeg mentioned this pull request Mar 19, 2019

Nominating Robert Fancsik (@rerobika) to become a JerryScript reviewer #2799

Closed


		jerry_typedarray_class_t klass = jerry_get_typedarray_type (typedarray);

		// klass is now JERRY_TYPEDARRAY_UINT32

Add TypedArray C API #2165

Add TypedArray C API #2165

Uh oh!

Conversation

galpeter commented Jan 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

galpeter commented Jan 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

galpeter commented Jan 30, 2018

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

galpeter commented Feb 5, 2018

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!