RFR: 8221541: clean up functions in CompilerOracle - was : RE: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?

[Doerr, Martin]

Hi Matthias,

looks good.

Best regards,
Martin


-----Original Message-----
From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of Vladimir Kozlov
Sent: Mittwoch, 27. März 2019 17:36
To: hotspot-dev at openjdk.java.net
Subject: Re: RFR: 8221541: clean up functions in CompilerOracle - was : RE: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?

Changes looks good.

It was part of debug functionality in C2 which was removed in jdk7u but adlc part was not:

http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/a5dd6e3ef9f3#l6.7

Thanks,
Vladimir

On 3/27/19 2:29 AM, Baesken, Matthias wrote:
> Hi Thomas, good point .
> I removed the 2 functions .
> Btw.  I checked them (using  nm )   in the  product   libjvm.so  and they showed up in the binary .
> So  it  might  help also  to get the binary a little bit smaller  …
> 
> 
> Please review !
> 
> Bug/webrev :
> 
> https://bugs.openjdk.java.net/browse/JDK-8221541
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8221541.0/
> 
> 
> Thanks, Matthias
> 
> 
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Sent: Dienstag, 26. März 2019 20:40
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: hotspot-dev at openjdk.java.net
> Subject: Re: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?
> 
> Hi Matthias,
> 
> On Tue, Mar 26, 2019 at 4:17 PM Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>> wrote:
> 
> Hello, it seems that the  fopen calls in CompilerOracle::append_comment_to_file and
> CompilerOracle::append_exclude_to_file  do not have a corresponding fclose.
> Or do I miss something ?
> 
> No, you are right, this is a bug. However, I think those functions are nowhere used, so maybe they could be removed completely.
> 
> Cheers, Thomas
> 
> We could also add a "true"  for the second argument of the fileStream constructor calls  ( fileStream(FILE* file, bool need_close = false)  )
> to get the closing.
> 
> 
> coding :
> ------------
> 
> jdk/src/hotspot/share/compiler/compilerOracle.cpp
> 
> 
> 741void CompilerOracle::append_comment_to_file(const char* message) {
> 742  assert(has_command_file(), "command file must be specified");
> 743  fileStream stream(fopen(cc_file(), "at"));
> 744  stream.print("# ");
> 745  for (int index = 0; message[index] != '\0'; index++) {
> 746    stream.put(message[index]);
> 747    if (message[index] == '\n') stream.print("# ");
> 748  }
> 749  stream.cr<http://stream.cr>();
> 750}
> 751
> 752void CompilerOracle::append_exclude_to_file(const methodHandle& method) {
> 753  assert(has_command_file(), "command file must be specified");
> 754  fileStream stream(fopen(cc_file(), "at"));
> 755  stream.print("exclude ");
> 756  method->method_holder()->name()->print_symbol_on(&stream);
> 757  stream.print(".");
> 758  method->name()->print_symbol_on(&stream);
> 759  method->signature()->print_symbol_on(&stream);
> 760  stream.cr<http://stream.cr>();
> 761  stream.cr<http://stream.cr>();
> 762}
> 
> Best regards, Matthias
>