RFR(S): 8201649: Remove dubious call_jio_print in ostream.cpp

Langer, Christoph christoph.langer at sap.com
Thu Apr 19 13:55:52 UTC 2018


Hi,

with my means of searching in the public OpenJDK repositories I can only find that 'call_jio_print' has always been there. No idea about the backgrounds of why we need this buffer copy approach instead of using jio_printf directly.

So, can someone else please help reviewing http://cr.openjdk.java.net/~clanger/webrevs/8201649.0/ ?

Thanks
Christoph

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Donnerstag, 19. April 2018 14:07
> To: Langer, Christoph <christoph.langer at sap.com>; Thomas Stüfe
> <thomas.stuefe at gmail.com>
> Cc: daniel.daugherty at oracle.com; hotspot-dev at openjdk.java.net; hotspot-
> runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8201649: Remove dubious call_jio_print in ostream.cpp
> 
> Hi Christoph,
> 
> On 19/04/2018 9:08 PM, Langer, Christoph wrote:
> > Hi Thomas, David,
> >
> >>> @David: No, I don't think it needs to be dialed back at this point, but I
> >> agree, the bug should be updated somewhat to the current scope by
> >> mentioning that the goal is to get rid of jio_print and jio_printf completely
> >> (along with call_jio_print in ostream.cpp). Those 2 functions were just 2
> >> hooks used in ostream.cpp and got routed through jio_vfprintf anyway,
> >> except for the raw write case when no vfprint_hook was installed - and
> we'll
> >> preserve that.
> 
> You preserved it in the sense it's still there, but you changed two call
> sites so they can never now hit it. So I'm back to the unanswerable
> "does that matter?".
> 
> Unless someone can do the archaeology on this I'm going to have to abstain.
> 
> David
> -----
> 
> >>>
> >>
> >> Hm. Instead of broading the scope of the issue, I would actually
> >> prefer the bug description to be clarified and limited to the original
> >> problem we tried to solve. Because that was a real valid problem.
> >>
> >> If only to get David's full review and not to confuse him further :)
> >
> > Ok, I agree. So I tried to clarify the bug a bit more in its original sense. And
> we'd be back to webrev #0.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8201649
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201649.0/
> >
> > The change of the webrev is missing a cast in line 2729 - (len needs to be
> casted int). This produces a build warning which turns into an error in some
> platforms. I'll have that fixed in the submitted version.
> >
> > For the further cleanup of jio_print methods I'll probably file another bug.
> >
> > As I think we already had a principal agreement about this proposal earlier,
> I would consider this reviewed by stuefe and dholmes if I don't hear back
> from you today. I would push this tomorrow after push to jdk-submit and
> further internal testing here at SAP.
> >
> > Thanks
> > Christoph
> >


More information about the hotspot-runtime-dev mailing list