From e7744efb7d36aafa35dc0ac678dd6038e46804a1 Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Thu, 14 Mar 2019 06:22:19 -0600 Subject: [PATCH] expander: avoid unneeded namespace reference This change avoids the stair-step effect that is depicted in the "current Racket -M" build plot from the January 2019 blog post about Racket on Chez Scheme. The stair step in that plot is a result of a combination of effects, but one key part is that the `.set-transformer!` linklet import (to support macro definitions) has a reference back to the namespace. While `.set-transformer!` normally would not be captured in any closure, `db/private/generic/prepared` creates a thread that causes the "prefix" part of a closure to be moved to a thread's runstack before it can be pruned by the GC. The stair-step problem happens only when running directly from machine-independent form, because that form is recompiled in a way that doesn't optimize away the unused `.set-transformer!` import. The change in this commit avoids a reference to the namespace in some cases where it will not be useful, which turns out to be sufficient to address the build problem. A more complete repair would be to change the compiler to pair a closure prefix on the runstack with a liveness mask. An even more complete repair is to switch to Racket CS. Racket CS is immune to the problem, even when running from machine-independent bytecode, because its closures do not keep extra references (with the tradeoff that there's less sharing). --- racket/src/expander/eval/module.rkt | 12 ++++++++++-- racket/src/racket/src/startup.inc | 25 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/racket/src/expander/eval/module.rkt b/racket/src/expander/eval/module.rkt index ca9254551d..220676f861 100644 --- a/racket/src/expander/eval/module.rkt +++ b/racket/src/expander/eval/module.rkt @@ -190,8 +190,16 @@ (define module-body-instance-instance (make-module-body-instance-instance - #:set-transformer! (lambda (name val) - (namespace-set-transformer! ns (sub1 phase-level) name val)))) + #:set-transformer! (cond + [(zero-phase? phase-level) + (lambda (name val) + (error 'define-syntax "should not happen at phase level 0"))] + [(zero-phase? (phase+ phase-shift phase-level)) + ;; No use for phase -1 bindings + (lambda (name val) (void))] + [else + (lambda (name val) + (namespace-set-transformer! ns (sub1 phase-level) name val))]))) (define (instantiate-body) (instantiate-linklet phase-linklet diff --git a/racket/src/racket/src/startup.inc b/racket/src/racket/src/startup.inc index 51e3106534..2cdd0218a6 100644 --- a/racket/src/racket/src/startup.inc +++ b/racket/src/racket/src/startup.inc @@ -37864,14 +37864,37 @@ static const char *startup_source = "(values))))" "(let-values(((module-body-instance-instance_0)" "(let-values(((temp52_0)" +"(if(zero-phase?" +" phase-level_0)" +"(let-values()" "(lambda(name_0" " val_0)" +"(begin" +" 'temp52" +"(error" +" 'define-syntax" +" \"should not happen at phase level 0\"))))" +"(if(zero-phase?" +"(phase+" +" phase-shift_0" +" phase-level_0))" +"(let-values()" +"(lambda(name_0" +" val_0)" +"(begin" +" 'temp52" +"(void))))" +"(let-values()" +"(lambda(name_0" +" val_0)" +"(begin" +" 'temp52" "(namespace-set-transformer!" " ns_2" "(sub1" " phase-level_0)" " name_0" -" val_0))))" +" val_0))))))))" "(make-module-body-instance-instance18.1" " temp52_0))))" "(let-values(((instantiate-body_0)"