[Sbcl-devel] gencgc: unprotect old space by regions, not pages

[Vitaly M. <v.m...@gm...>]

Hi!

unprotect_oldspace() calls mprotect() for every single page, whereas
it's possible to change permission attributes for multiple consequent
pages (region) at once. Each syscall on modern x86-64 CPUs costs
approx. 1000 ticks + losses from L1 caches invalidation.

This patch reduces amount of mprotect() calls from 35943 to 1053 on
evaluating this code (including SBCL boot time):

(let nil (setq a (make-list 10000000)) nil)

diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
index 5bd67c8..eded3d2 100644
--- a/src/runtime/gencgc.c
+++ b/src/runtime/gencgc.c
@@ -3245,6 +3245,8 @@ static void
 unprotect_oldspace(void)
 {
     page_index_t i;
+    void* addr = 0;
+    unsigned long len = 0;
 
     for (i = 0; i < last_free_page; i++) {
         if (page_allocated_p(i)
@@ -3257,11 +3259,22 @@ unprotect_oldspace(void)
             /* Remove any write-protection. We should be able to rely
              * on the write-protect flag to avoid redundant calls. */
             if (page_table[i].write_protected) {
-                os_protect(page_start, PAGE_BYTES, OS_VM_PROT_ALL);
+                if (addr == 0) { /* first page in region */
+                    addr = page_start;
+                    len = PAGE_BYTES;
+                } else if (addr + len == page_start) { /* another page in same region */
+                    len += PAGE_BYTES;
+                } else {
+                    os_protect(addr, len, OS_VM_PROT_ALL); /* first page in another region */
+                    addr = page_start;
+                    len = PAGE_BYTES;
+                }
                 page_table[i].write_protected = 0;
             }
         }
     }
+    if (addr)
+        os_protect(addr, len, OS_VM_PROT_ALL);
 }
 
 /* Work through all the pages and free any in from_space. This

-- 
wbr, Vitaly