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