Skip to content

Reduce the argument count of ecma_op_object_get_property_names#2598

Merged
LaszloLango merged 1 commit intojerryscript-project:masterfrom
rerobika:opt_get_property_names
Nov 15, 2018
Merged

Reduce the argument count of ecma_op_object_get_property_names#2598
LaszloLango merged 1 commit intojerryscript-project:masterfrom
rerobika:opt_get_property_names

Conversation

@rerobika
Copy link
Copy Markdown
Member

Introduce ECMA_LIST_ARRAY_INDICES[_ENUMBERABLE[_PROTOTYPE]] flags instead of passing 3 boolean arguments.

JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu

/**
* List array indices and enumerable properties.
*/
#define ECMA_LIST_ARRAY_INDICES_ENUMERABLE (ECMA_LIST_ARRAY_INDICES | ECMA_LIST_ENUMERABLE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some defines from this list is not used. Please remove them. We will introduce them if needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed them.

Introduce ECMA_LIST_ARRAY_INDICES[_ENUMBERABLE[_PROTOTYPE]] flags instead of passing 3 boolean arguments.

JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu
@rerobika rerobika force-pushed the opt_get_property_names branch from a692b03 to c02b7c4 Compare November 14, 2018 13:20
Copy link
Copy Markdown
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

bool is_enumerable_only, /**< true - exclude non-enumerable properties */
bool is_with_prototype_chain) /**< true - list properties from prototype chain,
* false - list only own properties */
uint32_t opts) /**< any combination of ecma_list_properties_options_t values */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not uint8_t?

Copy link
Copy Markdown
Member Author

@rerobika rerobika Nov 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since argument passing could happen via registers (which work with at least 32 bit operands), it would be a waste of 3 bytes if we limit it to uint8_t. However this procedure also could happen via pushing them into the stack where your suggestion is more profitable. I'm not insist to uint32_t but i think it's more time resistant choice.

Copy link
Copy Markdown
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LaszloLango LaszloLango merged commit 9535a23 into jerryscript-project:master Nov 15, 2018
legendecas added a commit to yodaos-project/ShadowNode that referenced this pull request Nov 26, 2018
yorkie pushed a commit to yodaos-project/ShadowNode that referenced this pull request Nov 26, 2018
@rerobika rerobika deleted the opt_get_property_names branch February 28, 2019 11:19
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.

3 participants