Hi Cheleswer,
Looks good. (While we've discussed it offline I haven't actually
offered a public review).
Just one more thing, and this concerns the original code not the change:
1880 st->print("Warning: Address: 0x%x, Size: %dK,
",p->pr_vaddr, p->pr_size/1024, p->pr_mapname);
We pass 3 params when we only print two items? The "p->pr_mapname", we
pass again on the next line where we do have the the "%s" in the format
string, so this one is unnecessary. 8-)
Thanks
Kevin
On 07/03/2016 10:07, Dmitry Samersoff wrote:
Cheleswer,
Looks good for me.
-Dmitry
On 2016-03-03 19:28, Cheleswer Sahu wrote:
Hi,
Please review the new code changes for this bug. I have removed " fstat()" call which
seems not safe to read the "/proc/map/self" file and have made some other changes too.
Please find the latest code in below link
Webrev link: http://cr.openjdk.java.net/~csahu/8146683/webrev.01/
Regards,
Cheleswer
-----Original Message-----
From: Dean Long
Sent: Wednesday, February 24, 2016 7:44 AM
To: Daniel Daugherty; Thomas Stüfe; Cheleswer Sahu
Cc: serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR[9u-dev]: 8146683: check_addr0 should be more efficient
On 2/23/2016 10:07 AM, Daniel D. Daugherty wrote:
On 2/23/16 5:56 AM, Thomas Stüfe wrote:
Hi Cheleswer,
On Tue, Feb 23, 2016 at 9:38 AM, Cheleswer Sahu
<cheleswer.s...@oracle.com>
wrote:
Hi,
Please review the code changes for
https://bugs.openjdk.java.net/browse/JDK-8146683 .
Webrev link: http://cr.openjdk.java.net/~csahu/8146683/
JBS Link: https://bugs.openjdk.java.net/browse/JDK-8146683
Bug Brief:
While investigating bug
https://bugs.openjdk.java.net/browse/JDK-8144483
it has been observed that "check_addr0() " function is not written
efficiently.
This function is trying to read each "prmap_t" entry in
"/proc/self/map"
file one by one which is time taking and can cause in long pauses.
Solution proposed:
Instead of reading each "prmap_t" entry in "/proc/self/map" file one
by one, read the entries in chunks. There can be many entries in "map"
file,
so I have decided to read these
in chunks of 100 "prmap_t" entries. Reading entries in chunks saves
a lot of read calls and results in smaller pause times.
Regards,
Cheleswer
We saw cases, especially on Solaris, where the content of a file in the
proc file system changed under us while we were reading it. So this
should
be kept in mind. The original code seems also broken in that regard,
because it assumes the ::read() call to read the full size of a prmap_t
entry, but it may theoretically read an incomplete entry.
As for your coding, you first estimate the size of the mapping file and
then use this to determine how many entries to read; but when
reading, this
information may already be stale. I think it would be more robust and
also
simpler to just try to read as many entries as possible - in chunks, to
make reading faster - until you get EOF.
Kind Regards, Thomas
I'm really sure that we've been down this road before. In particular,
Dmitry Samersoff has worked on this issue (or one very much like it)
before, but he is on vacation until the end of the month.
There was a similar issue with Linux /proc/self/maps, and whether it was
safe to read the file with buffered stdio or not. See 8009062 and 7017193.
dl
Dan
On Tue, Feb 23, 2016 at 9:38 AM, Cheleswer Sahu
<cheleswer.s...@oracle.com>
wrote:
Hi,
Please review the code changes for
https://bugs.openjdk.java.net/browse/JDK-8146683 .
Webrev link: http://cr.openjdk.java.net/~csahu/8146683/
JBS Link: https://bugs.openjdk.java.net/browse/JDK-8146683
Bug Brief:
While investigating bug
https://bugs.openjdk.java.net/browse/JDK-8144483
it has been observed that "check_addr0() " function is not written
efficiently.
This function is trying to read each "prmap_t" entry in
"/proc/self/map"
file one by one which is time taking and can cause in long pauses.
Solution proposed:
Instead of reading each "prmap_t" entry in "/proc/self/map" file one by
one, read the entries in chunks. There can be many entries in "map"
file,
so I have decided to read these
in chunks of 100 "prmap_t" entries. Reading entries in chunks saves
a lot
of read calls and results in smaller pause times.
Regards,
Cheleswer
