Hi Kim,
On 03/17/2016 11:33 PM, Kim Barrett wrote:
On Mar 17, 2016, at 2:44 AM, Robbin Ehn <robbin....@oracle.com> wrote:
On 03/16/2016 05:13 PM, Kim Barrett wrote:
On Mar 16, 2016, at 8:33 AM, Robbin Ehn <robbin....@oracle.com> wrote:
Hi, please review this small change.
This also change allocation methods.
Bug: https://bugs.openjdk.java.net/browse/JDK-8151993/
Webrev: http://cr.openjdk.java.net/~rehn/8151993/webrev/
Thanks!
/Robbin
------------------------------------------------------------------------------
src/share/vm/logging/log.hpp
Changing this:
138 char* newbuf = NEW_C_HEAP_ARRAY(char, newbuf_len, mtLogging);
to this:
137 char* newbuf = (char*) os::malloc(sizeof(char) * newbuf_len,
mtLogging);
New code is missing out of memory handling that was present in the old
code. New code will just try to use newbuf, with bad results if the
allocation failed.
Yes
New code is missing ASSERT-conditionalized PrintMallocFree support
that was present in the old. I don't know how important this is,
given that we also have PrintMalloc. (And I have to wonder why we
have both PrintMalloc (develop) and PrintMallocFree (notproduct).)
------------------------------------------------------------------------------
I wonder if this is even is the right path, we have 25 hpp including
allocation.inline.
The other ones seem to do their allocation from their inline.hpp file.
Making me believe that we should either ignore this or do log.inline.hpp.
Either way this patch should be dropped, thoughts?
I don't think ignoring is the right long-term approach, though is
probably ok in the short term. In particular, I think adding
log.inline.hpp would touch a lot of files, which seems like a bad
thing to do just now, with largish changes coming soon that would need
to be merged. And I'm not convinced log.inline.hpp is the right long
term approach anyway.
Ok,
Rather, I prefer Carsten's suggestion of an out of line slow-path
function in a .cpp. This approach is possible, even though we're
dealing with templates, by using type-erasure. The generic
(non-templated) slow-path function would need to receive a couple of
function pointers from the vwrite template, e.g. the prefix and puts
functions that are referenced by the present template code. The
function pointer types don't refer to template parameters, e.g. the
prefix function's type is
size_t (*prefix_fn)(char*, size_t)
and puts is
void (*puts_fn)(LogLevelType, const char*)
I wanted to avoid introducing a lot of things not directly related to
removing the include.
Thanks for your input!
/Robbin
