[erlang-patches] Allow the pool of hipe constants to grow at runtime

Mikael Pettersson mikpe@REDACTED
Wed Sep 1 13:43:43 CEST 2010


On Tue, 31 Aug 2010 10:38:33 +0200, Björn Gustavsson <bgustavsson@REDACTED> wrote:
> 2010/8/30 Paul Guyot <pguyot@REDACTED>:
> >
> > Mikael, I understand that the OTP team prefers git fetch command lines, b=
> ut you can see the patch in plain old diff format here:
> > http://github.com/pguyot/otp/commit/2f00cf20e222dd1ad5c0c48b3808821640ff5=
> 7a6.diff
> 
> We do accept patches inlined in emails, provided
> that they formatted in the same way as "git format-patch" does
> and that they are not white-space damaged. Such
> patches are easy to apply using "git am". Since it
> can be tricky to avoid white-space damaging inlined
> patches (depending on which email client you use),
> we only recommend sending inlined patches if you
> know how to do it properly.
> 
> (Note: We don't accept attached patches or inlined
> patches that don't have the commit message in the
> subject line and email body.)
> 
> The advantage of inlined patches are that they
> can be easily reviewed on the mailing list by
> just replying to the email and inserting the comments
> after the lines they refer to.

That is indeed the reason why I asked for an inline patch.
Other mailing lists like gcc-patches and linux-kernel work
that way: inline patches for review and testing, then git pulls.

Having said that, the 2nd version of the patch is very different
from the 1st, and almost all coding-style complaints I had are gone.
One minor nit though:

> --- a/erts/emulator/hipe/hipe_bif0.c
> +++ b/erts/emulator/hipe/hipe_bif0.c
> @@ -450,52 +450,13 @@ BIF_RETTYPE hipe_bifs_alloc_data_2(BIF_ALIST_2)
>  }
> 
>  /*
> - * Memory area for constant Erlang terms.
> - *
> - * These constants must not be forwarded by the gc.
> - * Therefore, the gc needs to be able to distinguish between
> - * collectible objects and constants. Unfortunately, an Erlang
> - * process' collectible objects are scattered around in two
> - * heaps and a list of message buffers, so testing "is X a
> - * collectible object?" can be expensive.
> - *
> - * Instead, constants are placed in a single contiguous area,
> - * which allows for an inexpensive "is X a constant?" test.
> - *
> - * XXX: Allow this area to be grown.
> + * Statistics on hipe constants: size of hipe constants, in words.

In documentation and comments it's "HiPE" not "hipe".
"hipe" should only be used in source code identifiers.

Removing hipe_bifs:show_literals/0 is Ok.  hipe_bifs:constants_size/0
must remain though as the HiPE test suite uses it.

So ACK, if you do the hipe -> HiPE fix in the comment above.

/Mikael


More information about the erlang-patches mailing list