> 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. 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*)
