Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread Yasumasa Suenaga
Hi David, On 2020/07/08 15:27, David Holmes wrote: Hi Yasumasa, On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote: Hi David, Serguei, Serguei, thank you for replying even though you are on vacaiton! I uploaded new webrev:    http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.07/    Diff fro

Re: RFR(S): 8247762: [aarch64] Timeout in .../HeapDumpTestWithActiveProcess.java due to inf. loop in AARCH64CurrentFrameGuess.run()

2020-07-08 Thread Andrew Haley
On 06/07/2020 20:54, Patric Hedlin wrote: > Andrew, > > Something you can live with? Totally. Great explanation too. -- Andrew Haley (he/him) Java Platform Lead Engineer Red Hat UK Ltd. https://keybase.io/andrewhaley EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR(S): 8247762: [aarch64] Timeout in .../HeapDumpTestWithActiveProcess.java due to inf. loop in AARCH64CurrentFrameGuess.run()

2020-07-08 Thread Patric Hedlin
Thanks for reviewing Andrew. /Patric On 2020-07-08 11:49, Andrew Haley wrote: On 06/07/2020 20:54, Patric Hedlin wrote: Andrew, Something you can live with? Totally. Great explanation too.

Re: RFR(M): 8247515: OSX pc_to_symbol() lookup does not work with core files

2020-07-08 Thread Kevin Walls
Sure -- I was thinking lowest_offset_from_sym initialising starting at a high positive integer (that would now be PTRDIFF_MAX I think) to save a comparison with e.g. -1, you can just check if the new offset is less than lowest_offset_from_sym With the ptrdiff_t change you made, this all look

Re: RFR(M): 8247272: SA ELF file support has never worked for 64-bit causing address to symbol name mapping to fail

2020-07-08 Thread Kevin Walls
Hi Chris -- This is a great story/history lesson. You could if you like, edit those comments in ElfFileParser.java so "Elf32_Addr" as they will contain either "Elf64_Addr or Elf32_Addr", similarly Elf64_Off.  The other Elf64 fields are the same as the 32 bit ones. Yes, the symbol fields are

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread David Holmes
On 8/07/2020 6:04 pm, Yasumasa Suenaga wrote: Hi David, On 2020/07/08 15:27, David Holmes wrote: Hi Yasumasa, On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote: Hi David, Serguei, Serguei, thank you for replying even though you are on vacaiton! I uploaded new webrev:    http://cr.openjdk.java.n

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread Yasumasa Suenaga
Thanks David! Yasumasa On 2020/07/08 21:29, David Holmes wrote: On 8/07/2020 6:04 pm, Yasumasa Suenaga wrote: Hi David, On 2020/07/08 15:27, David Holmes wrote: Hi Yasumasa, On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote: Hi David, Serguei, Serguei, thank you for replying even though you ar

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread Daniel D. Daugherty
> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.08/ src/hotspot/share/prims/jvmtiEnv.cpp     No comments. src/hotspot/share/prims/jvmtiEnvBase.cpp     L1159:   Thread *current_thread = Thread::current();     Please add "#ifdef ASSERT" above and "#endif" below since     current_

RFR [15] : 8249028 : clean up FileInstaller $test.src $cwd in vmTestbase_nsk_monitoring tests

2020-07-08 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev/8249028/webrev.00/ > 547 lines changed: 0 ins; 361 del; 186 mod; Hi all, could you please review the patch which removes `FileInstaller . .` jtreg action from :vmTestbase_nsk_monitoring tests? from the main issue(8204985): > all vmTestbase tests have '@run d

Re: RFR(M): 8247272: SA ELF file support has never worked for 64-bit causing address to symbol name mapping to fail

2020-07-08 Thread Chris Plummer
Hi Kevin, Thanks for the review. I'll add the additional Elf64_Addr and Elf64_Off comments. Probably the others should be updated too. Although they are the same size, they do have different names. For example: /* Type for a 16-bit quantity.  */ typedef uint16_t Elf32_Half; typedef uint16_t E

Re: RFR(M): 8247272: SA ELF file support has never worked for 64-bit causing address to symbol name mapping to fail

2020-07-08 Thread Chris Plummer
Webrev has been updated with the suggested comment changes. Note to new reviewers, look in webrev.00 first since it doesn't have the clutter of the comment changes, making it easier to see which lines actually have code changes. http://cr.openjdk.java.net/~cjplummer/8247272/webrev.01/index.htm

Re: RFR(M): 8247272: SA ELF file support has never worked for 64-bit causing address to symbol name mapping to fail

2020-07-08 Thread Kevin Walls
Thanks Chris, it's a bit of clutter, but truthful clutter. 8-) On 08/07/2020 20:26, Chris Plummer wrote: Webrev has been updated with the suggested comment changes. Note to new reviewers, look in webrev.00 first since it doesn't have the clutter of the comment changes, making it easier to see

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread Yasumasa Suenaga
Hi Dan, Thanks for your comment! I uploaded new webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/ Diff from previous webrev: http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524 I saw similar build errors in libOneGetThreadListStackTraces.cpp on Windows. This webrev fix

Re: RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

2020-07-08 Thread Yasumasa Suenaga
On 2020/07/09 9:25, Yasumasa Suenaga wrote: Hi Dan, Thanks for your comment! I uploaded new webrev:   http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/   Diff from previous webrev: http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524 Submit repo reported some build errors on ma

Re: RFR (S) 8247615: Initialize the bytes left for the heap sampler

2020-07-08 Thread Jean Christophe Beyler
Hello all, Any way to get two reviews of the new version: http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.01/ Thanks and have a great evening! Jc On Tue, Jun 30, 2020 at 12:47 AM Martin Buchholz wrote: > Looks good to me, but of course I'm not qualified to review. > > On Mon, Jun 29, 2020