Skip to content

Commit 1a05402

Browse files
svaaralasvaarala
committed
Strip 'leave as garbage' valstack policy
'Leave as undefined' seems to be the best overall value stack initialization policy. While 'leave as garbage' is marginally better in a few cases (mostly when refcounting is disabled) it's probably not worth keeping two policies around.
1 parent 5603ba8 commit 1a05402

4 files changed

Lines changed: 53 additions & 114 deletions

File tree

‎src/duk_api_stack.c‎

Lines changed: 45 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -340,25 +340,16 @@ DUK_EXTERNAL void duk_set_top(duk_context *ctx, duk_idx_t index) {
340340
duk_hthread *thr = (duk_hthread *) ctx;
341341
duk_uidx_t vs_size;
342342
duk_uidx_t vs_limit;
343-
duk_uidx_t count;
344343
duk_uidx_t uindex;
345-
duk_uidx_t offset;
346-
duk_uidx_t offset_orig;
347344
duk_tval *tv;
348-
duk_tval *tv_base;
349345

350346
DUK_ASSERT_CTX_VALID(ctx);
351347
DUK_ASSERT(DUK_INVALID_INDEX < 0);
352348

353349
DUK_ASSERT(thr->valstack_top >= thr->valstack_bottom);
354350
DUK_ASSERT(thr->valstack_end >= thr->valstack_bottom);
355351
vs_size = (duk_uidx_t) (thr->valstack_top - thr->valstack_bottom);
356-
#if defined(DUK_USE_PREFER_SIZE)
357352
vs_limit = (duk_uidx_t) (thr->valstack_end - thr->valstack_bottom);
358-
#else
359-
DUK_ASSERT((duk_size_t) (thr->valstack_end - thr->valstack_bottom) == thr->valstack_size);
360-
vs_limit = (duk_uidx_t) thr->valstack_size;
361-
#endif
362353

363354
if (index < 0) {
364355
/* Negative indices are always within allocated stack but
@@ -387,81 +378,51 @@ DUK_EXTERNAL void duk_set_top(duk_context *ctx, duk_idx_t index) {
387378
DUK_ASSERT(uindex <= vs_limit);
388379

389380
/* Handle change in value stack top. Respect value stack
390-
* initialization policy: garbage above top, or 'undefined'
391-
* above top. Byte offset arithmetic improves generated
392-
* code.
381+
* initialization policy: 'undefined' above top. Note that
382+
* DECREF may cause a side effect that reallocates valstack,
383+
* so must relookup after DECREF.
393384
*/
394385

395386
if (uindex >= vs_size) {
396387
/* Stack size increases or stays the same. */
397-
398-
#if defined(DUK_USE_VALSTACK_GARBAGE)
399-
offset = sizeof(duk_tval) * (uindex - vs_size);
400-
offset_orig = offset;
401-
tv_base = thr->valstack_top;
402-
while (offset > 0) {
403-
offset -= sizeof(duk_tval);
404-
tv = (duk_tval *) ((duk_uint8_t *) tv_base + offset); /* compute in byte offsets */
405-
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv);
406-
DUK_ASSERT(!DUK_TVAL_IS_HEAP_ALLOCATED(tv)); /* no need to incref */
407-
}
408-
thr->valstack_top = (duk_tval *) ((duk_uint8_t *) tv_base + offset_orig);
409-
#else
410388
#if defined(DUK_USE_ASSERTIONS)
389+
duk_uidx_t count;
390+
411391
count = uindex - vs_size;
412-
while (count > 0) {
392+
while (count != 0) {
413393
count--;
414394
tv = thr->valstack_top + count;
415395
DUK_ASSERT(DUK_TVAL_IS_UNDEFINED_ACTUAL(tv));
416396
}
417397
#endif
418398
thr->valstack_top = thr->valstack_bottom + uindex;
419-
#endif
420399
} else {
421400
/* Stack size decreases. */
422-
423-
/* XXX: Here it would be useful to have a DECREF macro which
424-
* doesn't need a NULL check, and does refzero queueing without
425-
* running the refzero algorithm. There would be no pointer
426-
* instability in this case, and code could be inlined. After
427-
* the loop, one call to refzero would be needed.
428-
*/
429-
430-
#if defined(DUK_USE_VALSTACK_GARBAGE)
431-
#if defined(DUK_USE_REFERENCE_COUNTING)
432-
count = vs_size - uindex;
433-
DUK_ASSERT(count > 0);
434-
while (count > 0) {
435-
count--;
436-
tv = --thr->valstack_top; /* tv -> value just before prev top value */
437-
DUK_ASSERT(tv >= thr->valstack_bottom);
438-
DUK_TVAL_DECREF_FAST(thr, tv); /* side effects */
439-
}
440-
#else
441-
thr->valstack_top = thr->valstack_bottom + uindex;
442-
#endif
443-
#else
444401
#if defined(DUK_USE_REFERENCE_COUNTING)
402+
duk_uidx_t count;
403+
445404
count = vs_size - uindex;
446405
DUK_ASSERT(count > 0);
447406
while (count > 0) {
448407
count--;
449-
tv = --thr->valstack_top; /* tv -> value just before prev top value */
408+
tv = --thr->valstack_top; /* tv -> value just before prev top value; must relookup */
450409
DUK_ASSERT(tv >= thr->valstack_bottom);
451410
DUK_TVAL_SET_UNDEFINED_ACTUAL_UPDREF(thr, tv); /* side effects */
452411
}
453-
#else
454-
offset = sizeof(duk_tval) * (vs_size - uindex);
455-
offset_orig = offset;
456-
tv_base = thr->valstack_top;
457-
while (offset > 0) {
458-
offset -= sizeof(duk_tval);
459-
tv = (duk_tval *) ((duk_uint8_t *) tv_base + offset); /* compute in byte offsets */
412+
#else /* DUK_USE_REFERENCE_COUNTING */
413+
duk_uidx_t count;
414+
duk_tval *tv_end;
415+
416+
count = vs_size - uindex;
417+
tv = thr->valstack_top;
418+
tv_end = tv - count;
419+
DUK_ASSERT(tv > tv_end);
420+
do {
421+
tv--;
460422
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv);
461-
}
462-
thr->valstack_top = thr->valstack_bottom + uindex;
463-
#endif
464-
#endif
423+
} while (tv != tv_end);
424+
thr->valstack_top = tv_end;
425+
#endif /* DUK_USE_REFERENCE_COUNTING */
465426
}
466427
}
467428

@@ -627,23 +588,21 @@ DUK_LOCAL duk_bool_t duk__resize_valstack(duk_context *ctx, duk_size_t new_size)
627588
(void *) thr->valstack, (void *) thr->valstack_end,
628589
(void *) thr->valstack_bottom, (void *) thr->valstack_top));
629590

630-
#if !defined(DUK_USE_VALSTACK_GARBAGE)
631-
/* init newly allocated slots (only) */
591+
/* Init newly allocated slots (only). */
632592
p = (duk_tval *) (void *) ((duk_uint8_t *) thr->valstack + old_end_offset_post);
633593
while (p < thr->valstack_end) {
634-
/* never executed if new size is smaller */
594+
/* Never executed if new size is smaller. */
635595
DUK_TVAL_SET_UNDEFINED_ACTUAL(p);
636596
p++;
637597
}
638598

639-
/* assertion check: we maintain elements above top in known state */
640-
#ifdef DUK_USE_ASSERTIONS
599+
/* Assert for value stack initialization policy. */
600+
#if defined(DUK_USE_ASSERTIONS)
641601
p = thr->valstack_top;
642602
while (p < thr->valstack_end) {
643603
DUK_ASSERT(DUK_TVAL_IS_UNDEFINED_ACTUAL(p));
644604
p++;
645605
}
646-
#endif
647606
#endif
648607

649608
return 1;
@@ -956,9 +915,7 @@ DUK_EXTERNAL void duk_replace(duk_context *ctx, duk_idx_t to_index) {
956915
*/
957916
DUK_TVAL_SET_TVAL(&tv_tmp, tv2);
958917
DUK_TVAL_SET_TVAL(tv2, tv1);
959-
#if !defined(DUK_USE_VALSTACK_GARBAGE)
960918
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv1);
961-
#endif
962919
thr->valstack_top--;
963920
DUK_TVAL_DECREF(thr, &tv_tmp); /* side effects */
964921
}
@@ -1012,9 +969,7 @@ DUK_EXTERNAL void duk_remove(duk_context *ctx, duk_idx_t index) {
1012969
nbytes = (duk_size_t) (((duk_uint8_t *) q) - ((duk_uint8_t *) p)); /* Note: 'q' is top-1 */
1013970
DUK_MEMMOVE(p, p + 1, nbytes); /* zero size not an issue: pointers are valid */
1014971

1015-
#if !defined(DUK_USE_VALSTACK_GARBAGE)
1016972
DUK_TVAL_SET_UNDEFINED_ACTUAL(q);
1017-
#endif
1018973
thr->valstack_top--;
1019974

1020975
#ifdef DUK_USE_REFERENCE_COUNTING
@@ -1075,18 +1030,14 @@ DUK_EXTERNAL void duk_xcopymove_raw(duk_context *to_ctx, duk_context *from_ctx,
10751030
to_thr->valstack_top = (duk_tval *) (void *) (((duk_uint8_t *) p) + nbytes);
10761031

10771032
if (is_copy) {
1078-
/* incref copies, keep originals */
1033+
/* Incref copies, keep originals. */
10791034
q = to_thr->valstack_top;
10801035
while (p < q) {
10811036
DUK_TVAL_INCREF(to_thr, p); /* no side effects */
10821037
p++;
10831038
}
10841039
} else {
1085-
/* no net refcount change */
1086-
#if defined(DUK_USE_VALSTACK_GARBAGE)
1087-
q = from_thr->valstack_top;
1088-
from_thr->valstack_top = (duk_tval *) (void *) (( (duk_uint8_t *) q) - nbytes);
1089-
#else
1040+
/* No net refcount change. */
10901041
p = from_thr->valstack_top;
10911042
q = (duk_tval *) (void *) (((duk_uint8_t *) p) - nbytes);
10921043
from_thr->valstack_top = q;
@@ -1096,7 +1047,6 @@ DUK_EXTERNAL void duk_xcopymove_raw(duk_context *to_ctx, duk_context *from_ctx,
10961047
DUK_TVAL_SET_UNDEFINED_ACTUAL(p);
10971048
/* XXX: fast primitive to set a bunch of values to UNDEFINED */
10981049
}
1099-
#endif
11001050
}
11011051
}
11021052

@@ -2948,13 +2898,16 @@ DUK_INTERNAL void duk_push_unused(duk_context *ctx) {
29482898

29492899
DUK_EXTERNAL void duk_push_undefined(duk_context *ctx) {
29502900
duk_hthread *thr;
2951-
duk_tval *tv_slot;
29522901

29532902
DUK_ASSERT_CTX_VALID(ctx);
29542903
thr = (duk_hthread *) ctx;
29552904
DUK__CHECK_SPACE();
2956-
tv_slot = thr->valstack_top++;
2957-
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv_slot);
2905+
2906+
/* Because value stack init policy is 'undefined above top',
2907+
* we don't need to write, just assert.
2908+
*/
2909+
thr->valstack_top++;
2910+
DUK_ASSERT(DUK_TVAL_IS_UNDEFINED_ACTUAL(thr->valstack_top - 1));
29582911
}
29592912

29602913
DUK_EXTERNAL void duk_push_null(duk_context *ctx) {
@@ -4191,6 +4144,7 @@ DUK_INTERNAL void duk_push_hobject_bidx(duk_context *ctx, duk_small_int_t builti
41914144

41924145
DUK_EXTERNAL void duk_pop_n(duk_context *ctx, duk_idx_t count) {
41934146
duk_hthread *thr = (duk_hthread *) ctx;
4147+
duk_tval *tv;
41944148

41954149
DUK_ASSERT_CTX_VALID(ctx);
41964150

@@ -4214,31 +4168,24 @@ DUK_EXTERNAL void duk_pop_n(duk_context *ctx, duk_idx_t count) {
42144168
* instability), inline code.
42154169
*/
42164170

4217-
#ifdef DUK_USE_REFERENCE_COUNTING
4218-
while (count > 0) {
4219-
duk_tval *tv;
4171+
/* XXX: optimize loops */
42204172

4173+
#if defined(DUK_USE_REFERENCE_COUNTING)
4174+
while (count > 0) {
4175+
count--;
42214176
tv = --thr->valstack_top; /* tv points to element just below prev top */
42224177
DUK_ASSERT(tv >= thr->valstack_bottom);
4223-
#if defined(DUK_USE_VALSTACK_GARBAGE)
4224-
DUK_TVAL_DECREF(thr, tv); /* side effects */
4225-
#else
42264178
DUK_TVAL_SET_UNDEFINED_ACTUAL_UPDREF(thr, tv); /* side effects */
4227-
#endif
4228-
count--;
42294179
}
42304180
#else
4231-
#if defined(DUK_USE_VALSTACK_GARBAGE)
4232-
thr->valstack_top -= count;
4233-
#else
4181+
tv = thr->valstack_top;
42344182
while (count > 0) {
4235-
duk_tval *tv;
4236-
tv = --thr->valstack_top;
4183+
count--;
4184+
tv--;
42374185
DUK_ASSERT(tv >= thr->valstack_bottom);
42384186
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv);
4239-
count--;
42404187
}
4241-
#endif
4188+
thr->valstack_top = tv;
42424189
#endif
42434190

42444191
DUK_ASSERT(thr->valstack_top >= thr->valstack_bottom);
@@ -4263,26 +4210,16 @@ DUK_EXTERNAL void duk_pop(duk_context *ctx) {
42634210
DUK_ERROR(thr, DUK_ERR_API_ERROR, DUK_STR_POP_TOO_MANY);
42644211
}
42654212

4266-
#ifdef DUK_USE_REFERENCE_COUNTING
42674213
tv = --thr->valstack_top; /* tv points to element just below prev top */
42684214
DUK_ASSERT(tv >= thr->valstack_bottom);
4269-
#if defined(DUK_USE_VALSTACK_GARBAGE)
4270-
DUK_TVAL_DECREF(thr, tv); /* side effects */
4271-
#else
4215+
#ifdef DUK_USE_REFERENCE_COUNTING
42724216
DUK_TVAL_SET_UNDEFINED_ACTUAL_UPDREF(thr, tv); /* side effects */
4273-
#endif
4274-
#else
4275-
#if defined(DUK_USE_VALSTACK_GARBAGE)
4276-
thr->valstack_top--;
42774217
#else
4278-
tv = --thr->valstack_top;
4279-
DUK_ASSERT(tv >= thr->valstack_bottom);
42804218
DUK_TVAL_SET_UNDEFINED_ACTUAL(tv);
4281-
#endif
42824219
#endif
42834220
DUK_ASSERT(thr->valstack_top >= thr->valstack_bottom);
42844221
}
4285-
#endif
4222+
#endif /* !DUK_USE_PREFER_SIZE */
42864223

42874224
DUK_EXTERNAL void duk_pop_2(duk_context *ctx) {
42884225
DUK_ASSERT_CTX_VALID(ctx);

‎src/duk_bi_global.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,8 @@ DUK_INTERNAL duk_ret_t duk_bi_global_object_print_helper(duk_context *ctx) {
727727
duk_file *f_out;
728728
#endif
729729

730+
DUK_UNREF(thr);
731+
730732
magic = duk_get_current_magic(ctx);
731733
DUK_UNREF(magic);
732734

‎src/duk_hthread.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,10 @@
156156
*/
157157

158158
#if defined(DUK_USE_PREFER_SIZE)
159-
#define DUK__ASSERT_CTX_VSSIZE(ctx) /*nop*/
159+
#define DUK_ASSERT_CTX_VSSIZE(ctx) /*nop*/
160160
#else
161-
#define DUK__ASSERT_CTX_VSSIZE(ctx) \
162-
DUK_ASSERT((duk_size_t) (((duk_hthread *) (ctx))->valstack_top - ((duk_hthread *) (ctx))->valstack) == \
161+
#define DUK_ASSERT_CTX_VSSIZE(ctx) \
162+
DUK_ASSERT((duk_size_t) (((duk_hthread *) (ctx))->valstack_end - ((duk_hthread *) (ctx))->valstack) == \
163163
((duk_hthread *) (ctx))->valstack_size)
164164
#endif
165165
#define DUK_ASSERT_CTX_VALID(ctx) do { \
@@ -173,7 +173,7 @@
173173
DUK_ASSERT(((duk_hthread *) (ctx))->valstack_top >= ((duk_hthread *) (ctx))->valstack); \
174174
DUK_ASSERT(((duk_hthread *) (ctx))->valstack_top >= ((duk_hthread *) (ctx))->valstack_bottom); \
175175
DUK_ASSERT(((duk_hthread *) (ctx))->valstack_end >= ((duk_hthread *) (ctx))->valstack_top); \
176-
DUK__ASSERT_CTX_VSSIZE((ctx)); \
176+
DUK_ASSERT_CTX_VSSIZE((ctx)); \
177177
} while (0)
178178

179179
/*
@@ -280,7 +280,7 @@ struct duk_hthread {
280280

281281
/* Value stack: these are expressed as pointers for faster stack manipulation.
282282
* [valstack,valstack_top[ is GC-reachable, [valstack_top,valstack_end[ is
283-
* uninitialized garbage.
283+
* not GC-reachable but kept initialized as 'undefined'.
284284
*/
285285
duk_tval *valstack; /* start of valstack allocation */
286286
duk_tval *valstack_end; /* end of valstack allocation (exclusive) */

‎src/duk_hthread_alloc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ DUK_INTERNAL duk_bool_t duk_hthread_init_stacks(duk_heap *heap, duk_hthread *thr
3838
thr->valstack_top = thr->valstack;
3939

4040
for (i = 0; i < DUK_VALSTACK_INITIAL_SIZE; i++) {
41-
DUK_TVAL_SET_UNDEFINED_UNUSED(&thr->valstack[i]);
41+
DUK_TVAL_SET_UNDEFINED_ACTUAL(&thr->valstack[i]);
4242
}
4343

4444
/* callstack */

0 commit comments

Comments
 (0)