From e447fb152bd2edd686aabf1e1a287a7d0b595731 Mon Sep 17 00:00:00 2001 From: Arseniy Alekseyev Date: Thu, 23 Jan 2020 16:49:05 +0000 Subject: [PATCH 1/2] Make `Weak.blit` and `Ephemeron.blit_key` faster --- Changes | 6 ++++++ runtime/caml/weak.h | 34 ++++++++++++++++++++++++---------- runtime/weak.c | 4 ++-- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/Changes b/Changes index 7356759a7eb1..25ab964eaeaf 100644 --- a/Changes +++ b/Changes @@ -34,6 +34,12 @@ Working version - #9233: Restore the bytecode stack after an allocation. (Stephen Dolan, review by Gabriel Scherer and Jacques-Henri Jourdan) +- #9259: Made `Ephemeron.blit_key` and `Weak.blit` faster. They are now + linear in the size of the range being copied instead of depending on the + total sizes of the ephemerons or weak arrays involved. + (Arseniy Alekseyev, design advice by Leo White, review by François Bobot + and Damien Doligez) + ### Code generation and optimizations: - #8637, #8805: Record debug info for each allocation diff --git a/runtime/caml/weak.h b/runtime/caml/weak.h index a6259764390e..ee884f6314ae 100644 --- a/runtime/caml/weak.h +++ b/runtime/caml/weak.h @@ -161,16 +161,18 @@ extern value caml_ephe_none; /* In the header, in order to let major_gc.c and weak.c see the body of the function */ -static inline void caml_ephe_clean (value v){ +static inline void caml_ephe_clean_partial (value v, + mlsize_t offset_start, + mlsize_t offset_end) { value child; int release_data = 0; - mlsize_t size, i; - header_t hd; + mlsize_t i; CAMLassert(caml_gc_phase == Phase_clean); + CAMLassert(2 <= offset_start + && offset_start <= offset_end + && offset_end <= Wosize_hd (Hd_val(v))); - hd = Hd_val (v); - size = Wosize_hd (hd); - for (i = 2; i < size; i++){ + for (i = offset_start; i < offset_end; i++){ child = Field (v, i); ephemeron_again: if (child != caml_ephe_none @@ -198,16 +200,28 @@ static inline void caml_ephe_clean (value v){ child = Field (v, 1); if(child != caml_ephe_none){ - if (release_data){ + if (release_data){ Field (v, 1) = caml_ephe_none; } else { - /* The mark phase must have marked it */ - CAMLassert( !(Is_block (child) && Is_in_heap (child) - && Is_white_val (child)) ); + /* If we scanned all the keys and the data field remains filled, + then the mark phase must have marked it */ + CAMLassert( !(offset_start == 2 && offset_end == Wosize_hd (Hd_val(v)) && + Is_block (child) && Is_in_heap (child) + && Is_white_val (child))); } } } +static inline void caml_ephe_clean (value v) { + mlsize_t size; + header_t hd; + hd = Hd_val (v); + size = Wosize_hd (hd); + + caml_ephe_clean_partial(v, 2, size); +} + + #endif /* CAML_INTERNALS */ #ifdef __cplusplus diff --git a/runtime/weak.c b/runtime/weak.c index 9fda2166d231..492d38780705 100644 --- a/runtime/weak.c +++ b/runtime/weak.c @@ -530,8 +530,8 @@ CAMLexport void caml_ephemeron_blit_key(value ars, mlsize_t offset_s, offset_d += CAML_EPHE_FIRST_KEY; if (caml_gc_phase == Phase_clean){ - caml_ephe_clean(ars); - caml_ephe_clean(ard); + caml_ephe_clean_partial(ars, offset_s, offset_s + length); + caml_ephe_clean_partial(ard, offset_d, offset_d + length); } if (offset_d < offset_s){ for (i = 0; i < length; i++){ From 5903aa0f70628e1695a3ca47432c3ec66be92837 Mon Sep 17 00:00:00 2001 From: Arseniy Alekseyev Date: Fri, 24 Jan 2020 12:15:09 +0000 Subject: [PATCH 2/2] Small optimization: don't clean the keys about to be overwritten --- runtime/caml/weak.h | 4 ++-- runtime/weak.c | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/runtime/caml/weak.h b/runtime/caml/weak.h index ee884f6314ae..007a17140c09 100644 --- a/runtime/caml/weak.h +++ b/runtime/caml/weak.h @@ -205,8 +205,8 @@ static inline void caml_ephe_clean_partial (value v, } else { /* If we scanned all the keys and the data field remains filled, then the mark phase must have marked it */ - CAMLassert( !(offset_start == 2 && offset_end == Wosize_hd (Hd_val(v)) && - Is_block (child) && Is_in_heap (child) + CAMLassert( !(offset_start == 2 && offset_end == Wosize_hd (Hd_val(v)) + && Is_block (child) && Is_in_heap (child) && Is_white_val (child))); } } diff --git a/runtime/weak.c b/runtime/weak.c index 492d38780705..062d0227e658 100644 --- a/runtime/weak.c +++ b/runtime/weak.c @@ -531,7 +531,11 @@ CAMLexport void caml_ephemeron_blit_key(value ars, mlsize_t offset_s, if (caml_gc_phase == Phase_clean){ caml_ephe_clean_partial(ars, offset_s, offset_s + length); - caml_ephe_clean_partial(ard, offset_d, offset_d + length); + /* We don't need to clean the keys that are about to be overwritten, + except where cleaning them could result in releasing the data, + which can't happen if data is already released. */ + if (Field (ard, CAML_EPHE_DATA_OFFSET) != caml_ephe_none) + caml_ephe_clean_partial(ard, offset_d, offset_d + length); } if (offset_d < offset_s){ for (i = 0; i < length; i++){