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

[Martin C. <cra...@co...>]

Vitaly Mayatskikh wrote on Thu, Mar 11, 2010 at 03:16:20PM +0100: 
> At Thu, 11 Mar 2010 14:36:27 +0100, Vitaly Mayatskikh wrote:
> > 
> > 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.
> 
> Or, perhaps, it's better to do like in write_protect_generation_pages(): 

This is cool, although not as much a speedup as I was hoping for.

It cuts off 9% of system time for a performance test run, which makes
for 0.1% total but that is for large GC thresholds.

In any case, this patch seems not to break anything and speeds things
up.  I'm in favor of committing it.

Martin

> diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
> index 5bd67c8..98c92c3 100644
> --- a/src/runtime/gencgc.c
> +++ b/src/runtime/gencgc.c
> @@ -3244,22 +3244,33 @@ scavenge_newspace_generation(generation_index_t generation)
>  static void
>  unprotect_oldspace(void)
>  {
> -    page_index_t i;
> +    page_index_t start;
>  
> -    for (i = 0; i < last_free_page; i++) {
> -        if (page_allocated_p(i)
> -            && (page_table[i].bytes_used != 0)
> -            && (page_table[i].gen == from_space)) {
> +    for (start = 0; start < last_free_page; start++) {
> +        if (page_allocated_p(start)
> +            && (page_table[start].bytes_used != 0)
> +            && (page_table[start].gen == from_space)) {
>              void *page_start;
> +            page_index_t last;
>  
> -            page_start = (void *)page_address(i);
> +            /* Note the page as unprotected in the page tables. */
> +            page_table[start].write_protected = 0;
>  
> -            /* 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);
> -                page_table[i].write_protected = 0;
> +            for (last = start + 1; last < last_free_page; last++) {
> +                if (!(page_allocated_p(last)
> +                      && (page_table[last].bytes_used != 0)
> +                      && (page_table[last].gen == from_space)))
> +                    break;
> +                page_table[last].write_protected = 0;
>              }
> +
> +            page_start = (void *)page_address(start);
> +
> +            os_protect(page_start,
> +                       npage_bytes(last - start),
> +                       OS_VM_PROT_ALL);
> +
> +            start = last;
>          }
>      }
>  }
> 
> -- 
> wbr, Vitaly
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Sbcl-devel mailing list
> Sbc...@li...
> https://lists.sourceforge.net/lists/listinfo/sbcl-devel

-- 
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Martin Cracauer <cra...@co...>   http://www.cons.org/cracauer/
FreeBSD - where you want to go, today.      http://www.freebsd.org/