hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <o.kalnichev...@dplanet.ch>
Subject Re: Of Wire logging, of Men and the God ;-)
Date Thu, 13 Feb 2003 19:12:12 GMT
Mike, 
Many thanks for your feedback. I did yet have a chance to look into your
modifications but I surely will 

Jeff, 
I understand your reluctance to sacrifice performance for a purely
cosmetic improvement. I cant say I disagree.

However, apart from logging issue there's a bit of a problem with the
way headers are parsed right now. Have a look


public static String readLine(InputStream inputStream) throws 
IOException {
LOG.trace("enter HttpConnection.readLine()");

StringBuffer buf = new StringBuffer();
int ch = inputStream.read();
while (ch >= 0) {
  if (ch == '\r') {
    ch = inputStream.read();
    if (ch == '\n') {
      break;
    } else {
      buf.append('\r');
    }
  }
  buf.append((char) ch);
  ch = inputStream.read();
}
...
return (buf.toString());
}


Firstly, there's end of stream check missing, but it's just a minor bug

    ch = inputStream.read();
    if (ch == -1) {
      break;
    }
    if (ch == '\n') {
      break;
    } else {
      buf.append('\r');
    }

Secondly, I am a bit unhappy with this line

  buf.append((char) ch);

that explicitly casts byte to char. Of course, we all know that HTTP
uses ASCII for non-content data such as header and is VEEEEEEEERY
unlikely to change to UTF-8 or anything of a sort. 

It's just a question of whether we do things right (even if it means
taking a bit of performance hit) or not. I strongly believe that we
should use byte[] to String (and vise versa) conversion routines
provided by HttpConstans consistently throughout the entire code base,
even though it may require a bit of buffering and hence a bit of
performance hit
 
Bottom line. Let's tackle problems step by step through a series of
small patches and if we see that wire logging cannot be made
comprehensive, so be it.

Personally I am a strong believer in making things work 100% or not
making them at all. I'd rather remove wire logging all together, or
restrict it to exclusively dealing with request/response headers, rather
than having it occasionally spit out some bits and pieces. 

This said, I am perfectly aware that in this world 95 (or 98) percent
solutions seems to be more successful ;-) So, I am not going to start
World War III if the wire logging is left as it is

Cheers

Oleg    



On Thu, 2003-02-13 at 07:30, Jeffrey Dever wrote:
> Haven't had time to review all of this, but I am a bit concerned over 
> any performance issues and buffering.  The wirelog is a good thing, but 
> there are other ways of getting a request/response log.  I wouldnot like 
> to see it excessively effect performance or resident set size, or add 
> too much complexity.
> 
> Also I find that the wirelog output conforms to the 10%/90% rule where 
> 90% of its troubleshooting value comes from 10% of its output, namely 
> the headers and request/response status lines.  The applications I have 
> written based on HttpClient would never enable the wirelog because the 
> output was too large (and could contain offensive binary data), so I 
> would just never enable it and use the applications logging system and 
> my own methods to write out the headers.  This should be a better 
> service provided by HttpClient.
> 
> We could just read/buffer/process/log headers in one shot in one logger, 
> and handle the body logging seperately.  The header logs might be part 
> of the normal operation of some applications where the body log would 
> only be used for headscratching debugging.  That might help a little bit 
> with the seperation of concerns that we are grappling here.
> 
> Jandalf.
> 
> 
> Michael Becke wrote:
> 
> > Well after looking at this further it seems one problem with this is 
> > that bytes get written one at a time to the input WireLog.  I made a 
> > few changes in HttpConnection and WireLogInputStream that pose a 
> > possible solution.  This version of the InputStream buffers content 
> > until a "\r\n" is read or until a certain number of bytes are read.  
> > This way wire log will always work even if content is read using the 
> > socket input stream.  Take a look.  These are just some other ideas.
> >
> > Mike
> >
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
> 


Mime
View raw message