[webkit-reviews] review denied: [Bug 5243] Numerous performance improvements to new FastMalloc to fix regression : [Attachment 4165] numerous optimizations to the new malloc

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 5243: Numerous performance improvements to new FastMalloc to fix regression
http://bugzilla.opendarwin.org/show_bug.cgi?id=5243

Attachment 4165: numerous optimizations to the new malloc
http://bugzilla.opendarwin.org/attachment.cgi?id=4165&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'm puzzled by the use of "config.h" -- did we discover that was the best
practice and better than a prefix file?

Also, it seems that if we do use "config.h" it needs to be the first include in
each file, because other headers might have inline code that depends on it, but
it instead seems to be the second include in every file. Is this intentional or
perhaps a bug in the script you used to add it?

To echo a complaint you've made to me many times, it was really a pain to
search through all the boilerplate changes to the other files to find the
substantive changes to review.

I'm concerned that changes to constants in the FastMalloc.cpp source file seem
to be near comments that have not been updated. Are there any comments here
that need to be updated?

The use of __attribute__((always_inline)) is not inside an ifdef. Lets instead
do this in a more-portable way using a macro and __GNUC__ ifdef as we do with
PRIVATE_INLINE.

I suggest that PRIVATE_INLINE use a KXMLCORE prefix since it's a macro and in a
header.

Should remove the FIXME comment talking about PPC.

Is there an easy way to catch the mistake of pairing fastMalloc with plain old
free or vice versa in a build where NDEBUG is not defined? If not, we should
try to come up with something.