Add an API to traverse objects by their associated native data #2236

gabrielschulhof · 2018-03-08T03:32:14Z

JerryScript-DCO-1.0-Signed-off-by: Gabriel Schulhof [email protected]

My use case here is the creation of weak references to objects.

zherczeg

Good patch. Some style issues.

zherczeg · 2018-03-08T04:53:58Z

docs/02.API-REFERENCE.md

+  if (find_data_p->match_data_p == data_p)
+  {
+
+    // If the object was found, acquire it and store it in the user data.


Remove this newline here.

There is no newline there. Github added it.

I don't understand. There is a newline in line 4648 above the comment. Why github would add it?

Oh, that newline. I thought you meant the one between "user" and "data". I'll remove it.

zherczeg · 2018-03-08T04:54:29Z

jerry-core/include/jerryscript-core.h

@@ -211,6 +211,10 @@ typedef bool (*jerry_object_property_foreach_t) (const jerry_value_t property_na
                                                 const jerry_value_t property_value,
                                                 void *user_data_p);

+typedef bool (*jerry_native_info_foreach_t) (const jerry_value_t object,


No comment for this type.

zherczeg · 2018-03-08T04:56:48Z

jerry-core/api/jerry.c

+    if (!ecma_is_lexical_environment (iter_p))
+    {
+      native_pointer_p = ecma_get_native_pointer_value (iter_p, LIT_INTERNAL_MAGIC_STRING_NATIVE_POINTER);
+      if (native_pointer_p != NULL &&


Wrong place of &&. Should go the the second line.

zherczeg · 2018-03-08T04:57:04Z

jerry-core/api/jerry.c

+      native_pointer_p = ecma_get_native_pointer_value (iter_p, LIT_INTERNAL_MAGIC_STRING_NATIVE_POINTER);
+      if (native_pointer_p != NULL &&
+          ((const jerry_object_native_info_t *) native_pointer_p->u.info_p) == native_info_p &&
+          !foreach_p (ecma_make_object_value (iter_p),


Wrong && and alignment.

gabrielschulhof · 2018-03-08T05:14:27Z

@zherczeg I have fixed the issues you mentioned.

gabrielschulhof · 2018-03-08T16:00:09Z

@zherczeg I have now removed that newline as well.

martijnthe · 2018-03-08T19:29:13Z

Ooooh, this is useful. Thanks for adding!

zherczeg · 2018-03-09T04:56:52Z

One more question: why the test is called test-weak-reference.c ? Shouldn't be a test-traverse-objects.c?

gabrielschulhof · 2018-03-09T12:45:11Z

@zherczeg it's called that because it also illustrates how one would implement a weak reference.

zherczeg · 2018-03-12T07:26:02Z

The name would be ok for an example, but this is a test which tests a particular API function. What do you think?

martijnthe · 2018-03-12T09:17:48Z

jerry-core/api/jerry.c

+    {
+      native_pointer_p = ecma_get_native_pointer_value (iter_p, LIT_INTERNAL_MAGIC_STRING_NATIVE_POINTER);
+      if (native_pointer_p != NULL
+          && ((const jerry_object_native_info_t *) native_pointer_p->u.info_p) == native_info_p


In our project we have various jerry_object_native_info_ts where the nativestructures they represent are subclasses of one another.

Say I have base class B and subclass S1 and S2, with the current API proposal, if I want to traverse all objects of the base class, I would have to call jerry_traverse_objects_by_native_info 3x, with the jerry_object_native_info_t * of B of S1 and of S2.

That makes me wonder whether it would make sense to split this function into a more primitive bool jerry_traverse_objects(jerry_native_info_foreach_t foreach_p, void *user_data_p) (and perhaps call it ...foreach... instead?)

jerry_traverse_objects_by_native_info could be implemented on top of that as a convenience function for the simple case.

OK. I'll do it that way.

gabrielschulhof · 2018-03-12T18:08:17Z

@zherczeg You're right. I will rename it.

gabrielschulhof · 2018-03-12T21:07:40Z

@zherczeg I have renamed the file and I have exposed the more basic traversal as @martijnthe requested.

martijnthe · 2018-03-13T08:46:09Z

docs/02.API-REFERENCE.md

+**Prototype**
+
+```c
+bool jerry_objects_foreach (jerry_objects_


Got interrupted? ;)

martijnthe · 2018-03-13T08:49:04Z

jerry-core/api/jerry.c

+  return native_pointer_p != NULL
+  && ((const jerry_object_native_info_t *) native_pointer_p->u.info_p) == iteration_info_p->native_info_p
+  ? iteration_info_p->foreach_p (value, native_pointer_p->data_p, iteration_info_p->user_data_p)
+  : true;


Could you write this such that it's a bit easier to grasp?

martijnthe · 2018-03-13T08:49:42Z

I have exposed the more basic traversal as @martijnthe requested.

Thanks!

gabrielschulhof · 2018-03-13T17:37:24Z

@martijnthe I have finished the documentation and unpacked the return value in the callback.

zherczeg · 2018-03-14T11:02:22Z

jerry-core/api/jerry.c

+  };
+
+  return jerry_objects_foreach (jerry_objects_foreach_by_native_info_callback, &iteration_info);
+} /* jerry_objects_foreach_by_native_info */


Maybe my impression but using the other function to implement this actually increase complexity rather than decrease it. I would just duplicate the code.

zherczeg · 2018-03-14T11:03:33Z

jerry-core/include/jerryscript-core.h

@@ -210,6 +210,18 @@ typedef jerry_value_t (*jerry_vm_exec_stop_callback_t) (void *user_p);
 typedef bool (*jerry_object_property_foreach_t) (const jerry_value_t property_name,
                                                 const jerry_value_t property_value,
                                                 void *user_data_p);
+/**
+ *


Missing comment.

JerryScript-DCO-1.0-Signed-off-by: Gabriel Schulhof [email protected]

gabrielschulhof · 2018-03-14T18:36:26Z

@zherczeg I have addressed your review comments.

gabrielschulhof · 2018-03-14T19:41:08Z

Could you please restart https://travis-ci.org/jerryscript-project/jerryscript/jobs/353479244?

zherczeg

LGTM

LaszloLango

LGTM

zherczeg requested changes Mar 8, 2018

View reviewed changes

gabrielschulhof force-pushed the reference-from-data branch from 36e7808 to 2d3707c Compare March 8, 2018 05:11

gabrielschulhof force-pushed the reference-from-data branch from 2d3707c to 4404d1f Compare March 8, 2018 15:56

martijnthe reviewed Mar 12, 2018

View reviewed changes

gabrielschulhof force-pushed the reference-from-data branch from 4404d1f to c0474f7 Compare March 12, 2018 21:06

martijnthe reviewed Mar 13, 2018

View reviewed changes

gabrielschulhof force-pushed the reference-from-data branch from c0474f7 to dcb1b8a Compare March 13, 2018 17:36

zherczeg reviewed Mar 14, 2018

View reviewed changes

Add an API to traverse objects by their associated native data

3bb63dc

JerryScript-DCO-1.0-Signed-off-by: Gabriel Schulhof [email protected]

gabrielschulhof force-pushed the reference-from-data branch from dcb1b8a to 3bb63dc Compare March 14, 2018 18:35

zherczeg approved these changes Mar 15, 2018

View reviewed changes

LaszloLango approved these changes Mar 21, 2018

View reviewed changes

LaszloLango merged commit 3664d9d into jerryscript-project:master Mar 21, 2018

gabrielschulhof deleted the reference-from-data branch March 21, 2018 14:06

martijnthe mentioned this pull request Mar 26, 2018

Garbage collection doesn't free values released during garbage collection (via native pointers) #2253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an API to traverse objects by their associated native data #2236

Add an API to traverse objects by their associated native data #2236

gabrielschulhof commented Mar 8, 2018

zherczeg left a comment

zherczeg Mar 8, 2018

gabrielschulhof Mar 8, 2018

zherczeg Mar 8, 2018

gabrielschulhof Mar 8, 2018

zherczeg Mar 8, 2018

zherczeg Mar 8, 2018

zherczeg Mar 8, 2018

gabrielschulhof commented Mar 8, 2018

gabrielschulhof commented Mar 8, 2018

martijnthe commented Mar 8, 2018

zherczeg commented Mar 9, 2018

gabrielschulhof commented Mar 9, 2018 •

edited

Loading

zherczeg commented Mar 12, 2018

martijnthe Mar 12, 2018

gabrielschulhof Mar 12, 2018

gabrielschulhof commented Mar 12, 2018

gabrielschulhof commented Mar 12, 2018

martijnthe Mar 13, 2018

martijnthe Mar 13, 2018

martijnthe commented Mar 13, 2018

gabrielschulhof commented Mar 13, 2018

zherczeg Mar 14, 2018

zherczeg Mar 14, 2018

gabrielschulhof commented Mar 14, 2018

gabrielschulhof commented Mar 14, 2018

zherczeg left a comment

LaszloLango left a comment

Add an API to traverse objects by their associated native data #2236

Add an API to traverse objects by their associated native data #2236

Conversation

gabrielschulhof commented Mar 8, 2018

zherczeg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrielschulhof commented Mar 8, 2018

gabrielschulhof commented Mar 8, 2018

martijnthe commented Mar 8, 2018

zherczeg commented Mar 9, 2018

gabrielschulhof commented Mar 9, 2018 • edited Loading

zherczeg commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrielschulhof commented Mar 12, 2018

gabrielschulhof commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martijnthe commented Mar 13, 2018

gabrielschulhof commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrielschulhof commented Mar 14, 2018

gabrielschulhof commented Mar 14, 2018

zherczeg left a comment

Choose a reason for hiding this comment

LaszloLango left a comment

Choose a reason for hiding this comment

gabrielschulhof commented Mar 9, 2018 •

edited

Loading