RFR(S): 8164039: Convert test_memset_with_concurrent_readers to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Tue Aug 23 13:02:02 UTC 2016


Kim,

Here are a new WebRev: 
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164039/webrev.04/

I fixed line breaks, moved std::ostringstream err_stream, re-alligned << 
and changed one "\n" to "std::endl"

Could you please let me know your opinion?

Thank you.

Regards, Kirill

On 22.08.2016 23:06, Kim Barrett wrote:
>> On Aug 22, 2016, at 6:42 AM, Kirill Zhaldybin <kirill.zhaldybin at oracle.com> wrote:
>>
>> Kim,
>>
>> Here are a new WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164039/webrev.03/
>> I changed output from printf to stream which then printed by ASSERT.
>>
>> Could you please let me know your opinion?
> Almost.  Some minor nits, mostly about formatting.
>
> test/native/gc/shared/test_memset_with_concurrent_readers.cpp
>    75         std::ostringstream err_stream;
>    76         if (!(head_clear && middle_set && tail_clear)) {
>
> err_stream is only used in the if-then body.  Move the declaration
> there, e.g. swap those two lines and adjust indentation.
>
> There are also some line breaks and indentation changes from the
> original that aren’t our normal style.  Sorry I didn't notice some of
> these earlier.
>
>    69         memset_with_concurrent_readers(&block[set_start], set_value,
>    70                 set_size);
>
> Remove line break.
>
>    73         bool tail_clear = !memcmp(clear_block, block + set_end,
>    74                 block_size - set_end);
>
> Remove line break.
>
>    77           err_stream << "*** memset_with_concurrent_readers failed: "
>    78                   "set start " << set_start << ", set end " << set_end << "\n";
>
> I think this would be simplified by eliminating the implicit string
> concatentation and appending "set start " to the end of the string on
> line 77.  Then align the "<<" on 78 with the one on 77.
>
>    79           for (unsigned chunk = 0; chunk < (block_size / chunk_size);
>    80                   ++chunk) {
>
> Remove line break.
>
>    81             for (unsigned line = 0; line < (chunk_size / BytesPerWord);
>    82                     ++line) {
>
> Remove line break.
>
>    84               const char* lp
>    85                       = &block[chunk * chunk_size + line * BytesPerWord];
>
> Remove line break.
>
>    87               err_stream << std::dec << chunk << "," << line << ": " << std::hex
>    88                       << std::setw(2) << line_byte(lp, 0) << " "
>    89                       << std::setw(2) << line_byte(lp, 1) << "  "
>    90                       << std::setw(2) << line_byte(lp, 2) << " "
>    91                       << std::setw(2) << line_byte(lp, 3) << "  "
>    92                       << std::setw(2) << line_byte(lp, 4) << " "
>    93                       << std::setw(2) << line_byte(lp, 5) << "  "
>    94                       << std::setw(2) << line_byte(lp, 6) << " "
>    95                       << std::setw(2) << line_byte(lp, 7) << std::endl;
>
> Align the leading "<<" on 88-95 with the first one on 87.
>
>
>




More information about the hotspot-gc-dev mailing list