RFR: JDK-8266503: [ul] Make Decorations safely copy-able and reduce their size

[Xin Liu]

On Tue, 4 May 2021 15:05:43 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> In Universal Logging, class LogDecorations keeps resolved decorations as well as a lookup table of pointers to the start of each resolved decoration, by decorator. This is dangerous, since it makes object copy non-trivial (the pointers would need to be relocated). It is also wasteful since we spend 8 bytes per pointer per index.
> 
> Better would be to use a numerical index array of offset values, which could be safely copied. 
> 
> And since the resolve buffer is 256 char, we can easily make this index an 8-bit value, which reduces the size of a LogDecorations object down to 280 bytes (from 368). Size matters especially in the context of JDK-8229517.
> 
> The patch also adds a gtest, which tests that decorations are safely copy-able and that decorator resolution works as expected.
> 
> Testing:
> - manually jtreg runtime/logging
> - manually gtests
> - Will run more tests tonight

I read this patch and PR#3874.  IMHO, I don't think PR#3874 is over this patch in terms of "makes the code easier to understand and read." The change is obviously bigger(+238 −117) than this one.  Is it worth it? 

To me, the only critical drawback of storing resolved c-string is i18n. As a matter of fact, The unified logging doesn't put i18n in consideration, so it is not an issue. 
 
A universal buffer seems cleaner than scatter fields. Reducing size(from 368 -> 56 bytes on Linux x64) is appealing, but it actually has very limited impact on real performance. 

I am not a reviewer. Those are all my personal opinions. Take it as a grain of salt. I can go either way.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3855