Skip to content

Rearrange fields of ecma_property_t to be naturally aligned. #904

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Rearrange fields of ecma_property_t to be naturally aligned. #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 82 additions & 91 deletions jerry-core/ecma/base/ecma-gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,130 +289,121 @@ ecma_gc_mark (ecma_object_t *object_p) /**< object to mark from */
next_property_p = ECMA_GET_POINTER (ecma_property_t,
property_p->next_property_p);

switch ((ecma_property_type_t) property_p->type)
if (property_p->flags & ECMA_PROPERTY_FLAG_NAMEDDATA)
{
case ECMA_PROPERTY_NAMEDDATA:
{
ecma_value_t value = ecma_get_named_data_property_value (property_p);
ecma_value_t value = ecma_get_named_data_property_value (property_p);

if (ecma_is_value_object (value))
{
ecma_object_t *value_obj_p = ecma_get_object_from_value (value);
if (ecma_is_value_object (value))
{
ecma_object_t *value_obj_p = ecma_get_object_from_value (value);

ecma_gc_set_object_visited (value_obj_p, true);
}
ecma_gc_set_object_visited (value_obj_p, true);
}
}
else if (property_p->flags & ECMA_PROPERTY_FLAG_NAMEDACCESSOR)
{
ecma_object_t *getter_obj_p = ecma_get_named_accessor_property_getter (property_p);
ecma_object_t *setter_obj_p = ecma_get_named_accessor_property_setter (property_p);

break;
if (getter_obj_p != NULL)
{
ecma_gc_set_object_visited (getter_obj_p, true);
}

case ECMA_PROPERTY_NAMEDACCESSOR:
if (setter_obj_p != NULL)
{
ecma_object_t *getter_obj_p = ecma_get_named_accessor_property_getter (property_p);
ecma_object_t *setter_obj_p = ecma_get_named_accessor_property_setter (property_p);
ecma_gc_set_object_visited (setter_obj_p, true);
}
}
else
{
JERRY_ASSERT (property_p->flags & ECMA_PROPERTY_FLAG_INTERNAL);

ecma_internal_property_id_t property_id = (ecma_internal_property_id_t) property_p->h.internal_property_type;
uint32_t property_value = property_p->v.internal_property.value;

if (getter_obj_p != NULL)
switch (property_id)
{
case ECMA_INTERNAL_PROPERTY_NUMBER_INDEXED_ARRAY_VALUES: /* a collection of ecma values */
case ECMA_INTERNAL_PROPERTY_STRING_INDEXED_ARRAY_VALUES: /* a collection of ecma values */
{
ecma_gc_set_object_visited (getter_obj_p, true);
JERRY_UNIMPLEMENTED ("Indexed array storage is not implemented yet.");
}

if (setter_obj_p != NULL)
case ECMA_INTERNAL_PROPERTY_PROTOTYPE: /* the property's value is located in ecma_object_t
(see above in the routine) */
case ECMA_INTERNAL_PROPERTY_EXTENSIBLE: /* the property's value is located in ecma_object_t
(see above in the routine) */
case ECMA_INTERNAL_PROPERTY__COUNT: /* not a real internal property type,
* but number of the real internal property types */
{
ecma_gc_set_object_visited (setter_obj_p, true);
JERRY_UNREACHABLE ();
}

break;
}

case ECMA_PROPERTY_INTERNAL:
{
ecma_internal_property_id_t property_id = (ecma_internal_property_id_t) property_p->u.internal_property.type;
uint32_t property_value = property_p->u.internal_property.value;
case ECMA_INTERNAL_PROPERTY_PRIMITIVE_STRING_VALUE: /* compressed pointer to a ecma_string_t */
case ECMA_INTERNAL_PROPERTY_PRIMITIVE_NUMBER_VALUE: /* compressed pointer to a ecma_number_t */
case ECMA_INTERNAL_PROPERTY_PRIMITIVE_BOOLEAN_VALUE: /* a simple boolean value */
case ECMA_INTERNAL_PROPERTY_CLASS: /* an enum */
case ECMA_INTERNAL_PROPERTY_CODE_BYTECODE: /* compressed pointer to a bytecode array */
case ECMA_INTERNAL_PROPERTY_NATIVE_CODE: /* an external pointer */
case ECMA_INTERNAL_PROPERTY_NATIVE_HANDLE: /* an external pointer */
case ECMA_INTERNAL_PROPERTY_FREE_CALLBACK: /* an object's native free callback */
case ECMA_INTERNAL_PROPERTY_BUILT_IN_ID: /* an integer */
case ECMA_INTERNAL_PROPERTY_BUILT_IN_ROUTINE_DESC: /* an integer */
case ECMA_INTERNAL_PROPERTY_EXTENSION_ID: /* an integer */
case ECMA_INTERNAL_PROPERTY_NON_INSTANTIATED_BUILT_IN_MASK_0_31: /* an integer (bit-mask) */
case ECMA_INTERNAL_PROPERTY_NON_INSTANTIATED_BUILT_IN_MASK_32_63: /* an integer (bit-mask) */
case ECMA_INTERNAL_PROPERTY_REGEXP_BYTECODE:
{
break;
}

switch (property_id)
case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_BOUND_THIS: /* an ecma value */
{
case ECMA_INTERNAL_PROPERTY_NUMBER_INDEXED_ARRAY_VALUES: /* a collection of ecma values */
case ECMA_INTERNAL_PROPERTY_STRING_INDEXED_ARRAY_VALUES: /* a collection of ecma values */
if (ecma_is_value_object (property_value))
{
JERRY_UNIMPLEMENTED ("Indexed array storage is not implemented yet.");
}
ecma_object_t *obj_p = ecma_get_object_from_value (property_value);

case ECMA_INTERNAL_PROPERTY_PROTOTYPE: /* the property's value is located in ecma_object_t
(see above in the routine) */
case ECMA_INTERNAL_PROPERTY_EXTENSIBLE: /* the property's value is located in ecma_object_t
(see above in the routine) */
case ECMA_INTERNAL_PROPERTY__COUNT: /* not a real internal property type,
* but number of the real internal property types */
{
JERRY_UNREACHABLE ();
}

case ECMA_INTERNAL_PROPERTY_PRIMITIVE_STRING_VALUE: /* compressed pointer to a ecma_string_t */
case ECMA_INTERNAL_PROPERTY_PRIMITIVE_NUMBER_VALUE: /* compressed pointer to a ecma_number_t */
case ECMA_INTERNAL_PROPERTY_PRIMITIVE_BOOLEAN_VALUE: /* a simple boolean value */
case ECMA_INTERNAL_PROPERTY_CLASS: /* an enum */
case ECMA_INTERNAL_PROPERTY_CODE_BYTECODE: /* compressed pointer to a bytecode array */
case ECMA_INTERNAL_PROPERTY_NATIVE_CODE: /* an external pointer */
case ECMA_INTERNAL_PROPERTY_NATIVE_HANDLE: /* an external pointer */
case ECMA_INTERNAL_PROPERTY_FREE_CALLBACK: /* an object's native free callback */
case ECMA_INTERNAL_PROPERTY_BUILT_IN_ID: /* an integer */
case ECMA_INTERNAL_PROPERTY_BUILT_IN_ROUTINE_DESC: /* an integer */
case ECMA_INTERNAL_PROPERTY_EXTENSION_ID: /* an integer */
case ECMA_INTERNAL_PROPERTY_NON_INSTANTIATED_BUILT_IN_MASK_0_31: /* an integer (bit-mask) */
case ECMA_INTERNAL_PROPERTY_NON_INSTANTIATED_BUILT_IN_MASK_32_63: /* an integer (bit-mask) */
case ECMA_INTERNAL_PROPERTY_REGEXP_BYTECODE:
{
break;
ecma_gc_set_object_visited (obj_p, true);
}

case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_BOUND_THIS: /* an ecma value */
{
if (ecma_is_value_object (property_value))
{
ecma_object_t *obj_p = ecma_get_object_from_value (property_value);
break;
}

ecma_gc_set_object_visited (obj_p, true);
}
case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_BOUND_ARGS: /* a collection of ecma values */
{
ecma_collection_header_t *bound_arg_list_p = ECMA_GET_NON_NULL_POINTER (ecma_collection_header_t,
property_value);

break;
}
ecma_collection_iterator_t bound_args_iterator;
ecma_collection_iterator_init (&bound_args_iterator, bound_arg_list_p);

case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_BOUND_ARGS: /* a collection of ecma values */
for (ecma_length_t i = 0; i < bound_arg_list_p->unit_number; i++)
{
ecma_collection_header_t *bound_arg_list_p = ECMA_GET_NON_NULL_POINTER (ecma_collection_header_t,
property_value);

ecma_collection_iterator_t bound_args_iterator;
ecma_collection_iterator_init (&bound_args_iterator, bound_arg_list_p);
bool is_moved = ecma_collection_iterator_next (&bound_args_iterator);
JERRY_ASSERT (is_moved);

for (ecma_length_t i = 0; i < bound_arg_list_p->unit_number; i++)
if (ecma_is_value_object (*bound_args_iterator.current_value_p))
{
bool is_moved = ecma_collection_iterator_next (&bound_args_iterator);
JERRY_ASSERT (is_moved);
ecma_object_t *obj_p = ecma_get_object_from_value (*bound_args_iterator.current_value_p);

if (ecma_is_value_object (*bound_args_iterator.current_value_p))
{
ecma_object_t *obj_p = ecma_get_object_from_value (*bound_args_iterator.current_value_p);

ecma_gc_set_object_visited (obj_p, true);
}
ecma_gc_set_object_visited (obj_p, true);
}

break;
}

case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_TARGET_FUNCTION: /* an object */
case ECMA_INTERNAL_PROPERTY_SCOPE: /* a lexical environment */
case ECMA_INTERNAL_PROPERTY_PARAMETERS_MAP: /* an object */
{
ecma_object_t *obj_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, property_value);
break;
}

ecma_gc_set_object_visited (obj_p, true);
case ECMA_INTERNAL_PROPERTY_BOUND_FUNCTION_TARGET_FUNCTION: /* an object */
case ECMA_INTERNAL_PROPERTY_SCOPE: /* a lexical environment */
case ECMA_INTERNAL_PROPERTY_PARAMETERS_MAP: /* an object */
{
ecma_object_t *obj_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, property_value);

break;
}
}
ecma_gc_set_object_visited (obj_p, true);

break;
break;
}
}
}
}
Expand Down
96 changes: 40 additions & 56 deletions jerry-core/ecma/base/ecma-globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,6 @@ typedef enum
ECMA_SIMPLE_VALUE__COUNT /** count of simple ecma values */
} ecma_simple_value_t;

/**
* Type of ecma-property
*/
typedef enum
{
ECMA_PROPERTY_NAMEDDATA, /**< named data property */
ECMA_PROPERTY_NAMEDACCESSOR, /**< named accessor property */
ECMA_PROPERTY_INTERNAL /**< internal property */
} ecma_property_type_t;

/**
* Description of an ecma value
*
Expand Down Expand Up @@ -213,84 +203,78 @@ typedef enum
} ecma_property_configurable_value_t;

/**
* Width of internal property type field's width
* Property's flag list.
*/
#define ECMA_PROPERTY_INTERNAL_PROPERTY_TYPE_WIDTH (5)
typedef enum
{
ECMA_PROPERTY_FLAG_NAMEDDATA = 1u << 0, /**< property is named data */
ECMA_PROPERTY_FLAG_NAMEDACCESSOR = 1u << 1, /**< property is named accessor */
ECMA_PROPERTY_FLAG_INTERNAL = 1u << 2, /**< property is internal property */
ECMA_PROPERTY_FLAG_CONFIGURABLE = 1u << 3, /**< property is configurable */
ECMA_PROPERTY_FLAG_ENUMERABLE = 1u << 4, /**< property is enumerable */
ECMA_PROPERTY_FLAG_WRITABLE = 1u << 5, /**< property is writable */
ECMA_PROPERTY_FLAG_LCACHED = 1u << 6, /**< property is lcached */
} ecma_property_flags_t;

/**
* Pair of pointers - to property's getter and setter
*/
typedef struct
{
__extension__ mem_cpointer_t getter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to getter object */
__extension__ mem_cpointer_t setter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to setter object */
mem_cpointer_t getter_p; /**< pointer to getter object */
mem_cpointer_t setter_p; /**< pointer to setter object */
} ecma_getter_setter_pointers_t;

/**
* Description of ecma-property
*/
typedef struct __attr_packed___ ecma_property_t
typedef struct ecma_property_t
{
/** Property's type (ecma_property_type_t) */
unsigned int type : 2;

/** Compressed pointer to next property */
__extension__ mem_cpointer_t next_property_p : ECMA_POINTER_FIELD_WIDTH;
mem_cpointer_t next_property_p;

/** Property's flags (ecma_property_flags_t) */
uint8_t flags;

/** Property's details (depending on Type) */
/** Property's header part (depending on Type) */
union
{
/** Description of named data property */
struct __attr_packed___ ecma_named_data_property_t
{
/** Value */
__extension__ ecma_value_t value : ECMA_VALUE_SIZE;
/** Named data property value upper bits */
uint8_t named_data_property_value_high;
/** Internal property type */
uint8_t internal_property_type;
} h;
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, header ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is the names will be too long, and in some cases it is easy to reach the 120 char.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then leave as is.


/** Property's value part (depending on Type) */
union
{
/** Description of named data property (second part) */
struct
{
/** Compressed pointer to property's name (pointer to String) */
__extension__ mem_cpointer_t name_p : ECMA_POINTER_FIELD_WIDTH;

/** Flag indicating whether the property is registered in LCache */
unsigned int is_lcached : 1;

/** Attribute 'Writable' (ecma_property_writable_value_t) */
unsigned int writable : 1;

/** Attribute 'Enumerable' (ecma_property_enumerable_value_t) */
unsigned int enumerable : 1;
mem_cpointer_t name_p;

/** Attribute 'Configurable' (ecma_property_configurable_value_t) */
unsigned int configurable : 1;
/** Lower 16 bits of value */
uint16_t value_low;
Copy link
Member

Choose a reason for hiding this comment

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

With this layout, we are expecting that the value of a named data property fits into 8+16 bits. Right now, this is true, but it would be good to check somehow that ECMA_VALUE_SIZE (which gets calculated from at least 2 components from different header files) is less than 24. (A static assert could work, but according to its documentation, they are not to be placed in headers.)

} named_data_property;

/** Description of named accessor property */
struct __attr_packed___ ecma_named_accessor_property_t
/** Description of named accessor property (second part) */
struct
{
/** Compressed pointer to property's name (pointer to String) */
__extension__ mem_cpointer_t name_p : ECMA_POINTER_FIELD_WIDTH;

/** Attribute 'Enumerable' (ecma_property_enumerable_value_t) */
unsigned int enumerable : 1;

/** Attribute 'Configurable' (ecma_property_configurable_value_t) */
unsigned int configurable : 1;

/** Flag indicating whether the property is registered in LCache */
unsigned int is_lcached : 1;
mem_cpointer_t name_p;

/** Compressed pointer to pair of pointers - to property's getter and setter */
__extension__ mem_cpointer_t getter_setter_pair_cp : ECMA_POINTER_FIELD_WIDTH;
mem_cpointer_t getter_setter_pair_cp;
} named_accessor_property;

/** Description of internal property */
struct __attr_packed___ ecma_internal_property_t
/** Description of internal property (second part) */
struct
{
/** Internal property's type */
unsigned int type : ECMA_PROPERTY_INTERNAL_PROPERTY_TYPE_WIDTH;

/** Value (may be a compressed pointer) */
uint32_t value;
} internal_property;
} u;
} v;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer a real name for this. value?

} ecma_property_t;

/**
Expand Down
Loading