Merge pull request ocaml#9259 from aalekseyev/faster-Weak.blit

Faster Weak.blit
corwin-of-amber · Feb 14, 2020 · b807931 · b807931
2 parents 648d7a4 + 59ca2a6
commit b807931
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 12 deletions.
diff --git a/Changes b/Changes
@@ -48,6 +48,12 @@ Working version
   requires 64-bit integers to be double-word aligned (autoconf regression)
   (David Allsopp, review by Sébastien Hinderer)
 
+- #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)
+
 - #9279: Memprof optimisation.
   (Stephen Dolan, review by Jacques-Henri Jourdan)
 

diff --git 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 */
-Caml_inline void caml_ephe_clean (value v){
+Caml_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 @@ Caml_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)));
       }
   }
 }
 
+Caml_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
@@ -530,8 +530,12 @@ 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);
+    /* 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++){