Refactor literal-storage to not use C++ features #791

rtakacs · 2015-12-22T18:16:30Z

This patch removes all C++ features from the literal-storage. The functionality is the same, just the code structure is different.

Measurement results:

testing board:  rpi2
master revision: 4f44071

section	master size	with patch
.note.gnu.build-id	36	36
.text	190012	189612
.rodata	8176	8124
.ARM.exidx	8	8
.init_array	4	4
.data	1096	1096
.bss	2363840	2363848
.comment	40	40
.ARM.attributes	47	17
Total	2563259	2562815

zherczeg · 2015-12-22T18:18:46Z

Nice job!

seanshpark · 2015-12-23T00:21:24Z

+1

sand1k · 2015-12-24T14:42:42Z

I've rebased this patch onto the latest master. Please, check https://github.com/Samsung/jerryscript/tree/refactor-lit-storage-rebased

rtakacs · 2015-12-29T18:56:14Z

Thank you Andrey! I've updated this PR.

rtakacs · 2015-12-29T21:29:50Z

Measurement results:

testing board:  rpi2
master revision: 50d124b

section	size (master)	size (patch)
.note.gnu.build-id	36	36
.text	191100	191052
.rodata	8344	8332
.ARM.exidx	8	8
.init_array	4	4
.data	1096	1096
.bss	2363776	2363784
.comment	40	40
.ARM.attributes	47	47
Total	2564451	2564399

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	112-> 112 (0.000)	2.979-> 2.98 (-0.034)
access-binary-trees.js	84-> 84 (0.000)	1.835-> 1.833 (0.109)
access-fannkuch.js	40-> 40 (0.000)	8.816-> 8.98 (-1.860)
access-nbody.js	48-> 48 (0.000)	4.15-> 4.206 (-1.349)
bitops-3bit-bits-in-byte.js	32-> 32 (0.000)	2.255-> 2.261 (-0.266)
bitops-bits-in-byte.js	32-> 32 (0.000)	3.041-> 3.019 (0.723)
bitops-bitwise-and.js	32-> 32 (0.000)	3.415-> 3.411 (0.117)
controlflow-recursive.js	220-> 220 (0.000)	1.545-> 1.555 (-0.647)
crypto-aes.js	124-> 124 (0.000)	5.478-> 5.33 (2.702)
crypto-md5.js	184-> 184 (0.000)	33.818->32.642 (3.477)
crypto-sha1.js	132-> 132 (0.000)	14.971->14.542 (2.865)
date-format-xparb.js	72-> 72 (0.000)	1.656-> 1.655 (0.060)
math-cordic.js	40-> 40 (0.000)	3.29-> 3.305 (-0.456)
math-partial-sums.js	32-> 32 (0.000)	1.909-> 1.889 (1.048)
math-spectral-norm.js	40-> 40 (0.000)	2.057-> 2.047 (0.486)
string-fasta.js	48-> 48 (0.000)	5.662-> 5.588 (1.307)
Geometric mean:	RSS reduction: 0%	Speed up: 0.528%

galpeter · 2016-01-05T14:29:55Z

jerry-core/lit/lit-literal-storage.cpp

-                                                                         * string */
+record_t *
+create_charset_record (record_set_t *rec_set_p, /**< recordset */
+                       const lit_utf8_byte_t *str, /**< string to be placed in the record */


Two things:

str -> str_p

imho 'placed in the record' -> 'placed into the record'

ruben-ayrapetyan · 2016-01-12T11:21:55Z

jerry-core/rcs/rcs-records.h

+extern bool rcs_record_is_equal (rcs_record_set_t *, rcs_record_t *, rcs_record_t *);
+extern bool rcs_record_is_equal_utf8 (rcs_record_set_t *, rcs_record_t *, const lit_utf8_byte_t *, lit_utf8_size_t);
+
+#endif


Usually, we put comments after #endif to describe condition, corresponding to code just above the #endif. For example, !RCS_RECORDS_H in the case.

ruben-ayrapetyan · 2016-01-12T11:34:01Z

Looks good to me

sand1k · 2016-01-12T13:14:49Z

jerry-core/lit/lit-literal-storage.h

-#endif /* JERRY_ENABLE_SNAPSHOT */
+extern rcs_record_t *lit_create_charset_literal (rcs_record_set_t *, const lit_utf8_byte_t *, lit_utf8_size_t);
+extern rcs_record_t *lit_create_magic_literal (rcs_record_set_t *, lit_magic_string_id_t);
+extern rcs_record_t *lit_create_magic_record_ex (rcs_record_set_t *, lit_magic_string_ex_id_t);


lit_create_magic_literal_ex

galpeter · 2016-01-12T13:56:01Z

jerry-core/lit/lit-snapshot.h

+                                 uint32_t *);
+#endif /* JERRY_ENABLE_SNAPSHOT */
+
+#endif /* RCS_SNAPSHOT_H */


Shouldn't this be !RCS_SNAPSHOT_H ?

Yes it should, I missed that.

galpeter · 2016-01-12T14:10:27Z

jerry-core/rcs/rcs-records.cpp

+  JERRY_ASSERT (RCS_RECORD_IS_MAGIC_STR_EX (rec_p));
+
+  rcs_record_set_field (rec_p, RCS_MAGIC_STR_HEADER_ID_POS, RCS_MAGIC_STR_HEADER_ID_WIDTH, id);
+} /* rcs_record_set_magic_str_id */


This should be: rcs_record_set_magic_str_ex_id

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs [email protected]

galpeter · 2016-01-13T11:43:23Z

lgtm

galpeter · 2016-01-13T14:48:33Z

merged: 21f561f

jiangzidong · 2016-01-14T08:23:25Z

@galpeter @sand1k and @ruben-ayrapetyan
Is Jerry going to remove all c++ features and become a pure C project?

tilmannOSG · 2016-01-14T12:28:50Z

@jiangzidong Yes, that's correct. Several months ago a decision was made to move back to C and we are now incrementally rewriting the remaining C++ parts of JerryScript in C.

jiangzidong · 2016-01-14T12:33:50Z

@tilmannOSG Thanks. Do you have a schedule about when to complete the refactor

tilmannOSG · 2016-01-14T12:40:04Z

@jiangzidong We don't have a strict schedule but we aim to complete the move to C in the coming months.

jiangzidong · 2016-01-18T02:35:37Z

@tilmannOSG I see, and I am working on removing c++ features of the rcs_chunked_list

tilmannOSG · 2016-01-19T09:14:28Z

@jiangzidong That's great :) Contributions in that area are very welcome!

rtakacs force-pushed the refactor-lit-storage branch from 46efb89 to 5219470 Compare December 23, 2015 10:39

rtakacs force-pushed the refactor-lit-storage branch from 5219470 to 72c64df Compare December 29, 2015 18:53

LaszloLango added this to the Engine optimization & enhancement milestone Jan 5, 2016

LaszloLango assigned ruben-ayrapetyan Jan 5, 2016

galpeter reviewed Jan 5, 2016
View reviewed changes

ruben-ayrapetyan reviewed Jan 12, 2016
View reviewed changes

rtakacs force-pushed the refactor-lit-storage branch 3 times, most recently from c94f0a7 to 00379d6 Compare January 12, 2016 12:14

sand1k reviewed Jan 12, 2016
View reviewed changes

rtakacs force-pushed the refactor-lit-storage branch from 00379d6 to f138345 Compare January 12, 2016 13:40

galpeter reviewed Jan 12, 2016
View reviewed changes

rtakacs force-pushed the refactor-lit-storage branch from f138345 to 9dec857 Compare January 12, 2016 14:09

galpeter reviewed Jan 12, 2016
View reviewed changes

Refactor literal-storage to not use C++ features

a997b59

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs [email protected]

rtakacs force-pushed the refactor-lit-storage branch from 9dec857 to a997b59 Compare January 13, 2016 07:50

ruben-ayrapetyan assigned rtakacs and unassigned ruben-ayrapetyan Jan 13, 2016

galpeter closed this Jan 13, 2016

This was referenced Jan 14, 2016

Eliminate templates from jrt_read_from_buffer_by_offset and jrt_write_to_buffer_by_offset #708

Closed

Use rcs_record_iterator_t::skip(size_t) instead of its template version. #709

Closed

rtakacs mentioned this pull request Jan 18, 2016

refactor rcs_chunked_list and remove its c++ features #806

Closed

rtakacs deleted the refactor-lit-storage branch January 18, 2019 14:29

Refactor literal-storage to not use C++ features #791

Refactor literal-storage to not use C++ features #791

Uh oh!

Conversation

rtakacs commented Dec 22, 2015

Uh oh!

zherczeg commented Dec 22, 2015

Uh oh!

seanshpark commented Dec 23, 2015

Uh oh!

sand1k commented Dec 24, 2015

Uh oh!

rtakacs commented Dec 29, 2015

Uh oh!

rtakacs commented Dec 29, 2015

Uh oh!

galpeter Jan 5, 2016

Choose a reason for hiding this comment

Uh oh!

ruben-ayrapetyan Jan 12, 2016

Choose a reason for hiding this comment

Uh oh!

ruben-ayrapetyan commented Jan 12, 2016

Uh oh!

sand1k Jan 12, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter Jan 12, 2016

Choose a reason for hiding this comment

Uh oh!

rtakacs Jan 12, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter Jan 12, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter commented Jan 13, 2016

Uh oh!

galpeter commented Jan 13, 2016

Uh oh!

jiangzidong commented Jan 14, 2016

Uh oh!

tilmannOSG commented Jan 14, 2016

Uh oh!

jiangzidong commented Jan 14, 2016

Uh oh!

tilmannOSG commented Jan 14, 2016

Uh oh!

jiangzidong commented Jan 18, 2016

Uh oh!

tilmannOSG commented Jan 19, 2016

Uh oh!

Uh oh!