From owner-freebsd-current@FreeBSD.ORG Mon Nov 12 01:30:10 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D2A29816; Mon, 12 Nov 2012 01:30:10 +0000 (UTC) (envelope-from peter@rulingia.com) Received: from vps.rulingia.com (host-122-100-2-194.octopus.com.au [122.100.2.194]) by mx1.freebsd.org (Postfix) with ESMTP id 3E1098FC08; Mon, 12 Nov 2012 01:30:09 +0000 (UTC) Received: from server.rulingia.com (c220-239-241-202.belrs5.nsw.optusnet.com.au [220.239.241.202]) by vps.rulingia.com (8.14.5/8.14.5) with ESMTP id qAC1U6Rq064936 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 12 Nov 2012 12:30:07 +1100 (EST) (envelope-from peter@rulingia.com) X-Bogosity: Ham, spamicity=0.000000 Received: from server.rulingia.com (localhost.rulingia.com [127.0.0.1]) by server.rulingia.com (8.14.5/8.14.5) with ESMTP id qAC1U08p066416 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 12 Nov 2012 12:30:00 +1100 (EST) (envelope-from peter@server.rulingia.com) Received: (from peter@localhost) by server.rulingia.com (8.14.5/8.14.5/Submit) id qAC1U0RR066411; Mon, 12 Nov 2012 12:30:00 +1100 (EST) (envelope-from peter) Date: Mon, 12 Nov 2012 12:30:00 +1100 From: Peter Jeremy To: Devin Teske Subject: Re: HEADS UP: Forth Optimizations Message-ID: <20121112013000.GD5594@server.rulingia.com> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="i0/AhcQY5QxfSsSZ" Content-Disposition: inline In-Reply-To: X-PGP-Key: http://www.rulingia.com/keys/peter.pgp User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Adrian Chadd , freebsd-current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Nov 2012 01:30:10 -0000 --i0/AhcQY5QxfSsSZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2012-Nov-10 16:53:10 -0800, Devin Teske wrot= e: >Can someone help review this for the commit log? I've had a look through the proposed patch and my comments follow. Other than that, it looks good to me. >Index: menu-commands.4th >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >--- menu-commands.4th (revision 242835) >+++ menu-commands.4th (working copy) =2E.. >@@ -185,21 +240,21 @@ variable root_state =2E.. > s" set kernel=3D${kernel_prefix}${kernel[N]}${kernel_suffix}" >- \ command to assemble full kernel-path >- -rot tuck 36 + c! swap \ replace 'N' with array index value >- evaluate \ sets $kernel to full kernel-path >+ 36 +c! \ replace 'N' with ASCII numeral >+ evaluate I think the "sets $kernel to full kernel-path" comment is worth keeping. > s" set root=3D${root_prefix}${root[N]}${root_suffix}" >- \ command to assemble root image-path >- -rot tuck 30 + c! swap \ replace 'N' with array index value >- evaluate \ sets $kernel to full kernel-path >+ 30 +c! \ replace 'N' with ASCII numeral >+ evaluate Likewise, this could do with a (corrected) comment that it sets $root to the full path to root. >Index: menu.4th >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >--- menu.4th (revision 242835) >+++ menu.4th (working copy) >@@ -184,18 +223,15 @@ create init_text8 255 allot >=20 > \ base name of environment variable > loader_color? if >- s" ansi_caption[x]" >+ dup ansi_caption[x] > else >- s" menu_caption[x]" >+ dup menu_caption[x] > then=09 Could this be simplified to =3D dup =3D loader_color? if =3D ansi_caption[x] =3D else =3D menu_caption[x] =3D then=09 Or, at a higher level, should this whole block be pulled into a new word (along with similar words for toggled_{ansi,text}[x] and {ansi,menu}_caption[x][y]? >@@ -227,36 +263,26 @@ create init_text8 255 allot =2E.. > getenv dup -1 <> if > \ Assign toggled text to menu caption Some comments on stack contents around here would make it somewhat easier to follow what is going on. >@@ -329,19 +340,18 @@ create init_text8 255 allot =2E.. > \ This is highly unlikely to occur, but to make > \ sure that things move along smoothly, allocate > \ a temporary NULL string >=20 >+ drop ( getenv cruft ) > s" " > then > then Is this the memory leak? If so, can I suggest that this be commited separately since it is a simple change and is distinct from the other changes you are proposing. >@@ -357,14 +367,14 @@ create init_text8 255 allot > \=20 > \ Let's perform what we need to with the above. >=20 >- \ base name of menuitem caption var >+ \ Assign array value text to menu caption >+ 4 pick According to the docementation just above this hunk, there are only 4 items on the stack, so "4 pick" seems wrong, though it is consistent with my understanding of the old code. The "2 pick [char] 0" you added earlier seems to similarly be out-by-one, though consistent. >@@ -521,17 +528,20 @@ create init_text8 255 allot >=20 > \ If this is the ACPI menu option, act accordingly. > dup menuacpi @ =3D if >- acpimenuitem ( -- C-Addr/U | -1 ) >+ dup acpimenuitem ( n -- n n c-addr/u | n n -1 ) >+ dup -1 <> if >+ 13 +c! ( n n c-addr/u -- n ) \ replace 'x' I think the stack here should be ( n n c-addr/u -- n c-addr/u ) >@@ -950,100 +914,43 @@ create init_text8 255 allot >=20 > 49 \ Iterator start (loop range 49 to 56; ASCII '1' to '8') > begin >- \ Unset variables in-order of appearance in menu.4th(8) Does the order matter? I notice you've changed it. --i0/AhcQY5QxfSsSZ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlCgURgACgkQ/opHv/APuIdTRACgiVYnFNPNRSzqfiA5zoXv8/3K 06wAn1/tM3gsHOwCoeRuRKp5SgE8uzsS =hm9F -----END PGP SIGNATURE----- --i0/AhcQY5QxfSsSZ--