Re: [PATCH] http11: ~6% performance increase in header parsing

["ry dahl" <ry-Xek56AhD01PHviPkdFu9cA@public.gmane.org>]

This is broken in Ebb too. I'll put some time into it again today and
see if I can fix it.
ry

On Tue, Mar 25, 2008 at 5:53 AM, Evan Weaver <evan-72XWLPH10WVXUHR/Jj/Uug@public.gmane.org> wrote:
> >  I never committed the Ragel 6 upgrade to trunk because I couldn't figure
>  >  out why it was broken.  I was hoping someone else would step in and fix
>  >  it for us :)
>
>  Maybe Ry knows. Otherwise, I'll try to hack through it. I think
>  something might have to happen in the state transitions to make up for
>  the inability to explicitly close the output (is that even remotely
>  close?).
>
>
>  >  Digging into the revision history (with git :), the nasty_pound_header
>  >  test was committed in its commented form in r361 and has never changed
>  >  until now.
>
>  I'm not surprised, but there was no comment as to why it was disabled,
>  so I enabled it and let it fail. We should maybe remove that test
>  entirely and document multiline headers as not supported.
>
>  Thanks for looking in to this.
>
>
>
>  Evan
>
>
>
>  >  >
>  >  > On Sat, Mar 8, 2008 at 6:02 PM, Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org> wrote:
>  >  > > Evan Weaver <evan-72XWLPH10WVXUHR/Jj/Uug@public.gmane.org> wrote:
>  >  > >
>  >  > > > Upgrading ragel is fine with me.
>  >  > >
>  >  > >  Here's my work-in-progress.  The changes below are pretty much copied
>  >  > >  from ry's changes in ebb.
>  >  > >
>  >  > >  I haven't been able to figure out why
>  >  > >  test_horrible_queries(HttpParserTest) is failing, so any help here would
>  >  > >  be appreciated.
>  >  > >
>  >  > >  Thanks.
>  >  > >
>  >  > >  diff --git a/Rakefile b/Rakefile
>  >  > >  index f47d7a2..07a89c2 100644
>  >  > >  --- a/Rakefile
>  >  > >  +++ b/Rakefile
>  >  > >  @@ -55,13 +55,15 @@ task :ragel do
>  >  > >    Dir.chdir "ext/http11" do
>  >  > >      target = "http11_parser.c"
>  >  > >      File.unlink target if File.exist? target
>  >  > >  -    sh "ragel http11_parser.rl | rlgen-cd -G2 -o #{target}"
>  >  > >  +    # sh "ragel http11_parser.rl | rlgen-cd -G2 -o #{target}" # ragel 5.x
>  >  > >  +    sh "ragel -G2 http11_parser.rl" # ragel 6.0
>  >  > >      raise "Failed to build C source" unless File.exist? target
>  >  > >    end
>  >  > >    Dir.chdir "ext/http11" do
>  >  > >      target = "../../ext/http11_java/org/jruby/mongrel/Http11Parser.java"
>  >  > >      File.unlink target if File.exist? target
>  >  > >  -    sh "ragel -J http11_parser.java.rl | rlgen-java -o #{target}"
>  >  > >  +    # sh "ragel -J http11_parser.java.rl | rlgen-java -o #{target}" # ragel 5.x
>  >  > >  +    sh "ragel -J -o #{target} http11_parser.java.rl" # ragel 6.0
>  >  > >      raise "Failed to build Java source" unless File.exist? target
>  >  > >    end
>  >  > >   end
>  >  > >  diff --git a/ext/http11/http11_parser.rl b/ext/http11/http11_parser.rl
>  >  > >  index a418605..0c4e2d4 100644
>  >  > >  --- a/ext/http11/http11_parser.rl
>  >  > >  +++ b/ext/http11/http11_parser.rl
>  >  > >  @@ -114,7 +114,7 @@ size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len,
>  >  > >    p = buffer+off;
>  >  > >    pe = buffer+len;
>  >  > >
>  >  > >  -  assert(*pe == '\0' && "pointer does not end on NUL");
>  >  > >  +  /* assert(*pe == '\0' && "pointer does not end on NUL"); */
>  >  > >    assert(pe - p == len - off && "pointers aren't same distance");
>  >  > >
>  >  > >
>  >  > >  @@ -130,23 +130,11 @@ size_t http_parser_execute(http_parser *parser, const char *buffer, size_t len,
>  >  > >    assert(parser->field_len <= len && "field has length longer than whole buffer");
>  >  > >    assert(parser->field_start < len && "field starts after buffer end");
>  >  > >
>  >  > >  -  if(parser->body_start) {
>  >  > >  -    /* final \r\n combo encountered so stop right here */
>  >  > >  -    %%write eof;
>  >  > >  -    parser->nread++;
>  >  > >  -  }
>  >  > >  -
>  >  > >    return(parser->nread);
>  >  > >   }
>  >  > >
>  >  > >   int http_parser_finish(http_parser *parser)
>  >  > >   {
>  >  > >  -  int cs = parser->cs;
>  >  > >  -
>  >  > >  -  %%write eof;
>  >  > >  -
>  >  > >  -  parser->cs = cs;
>  >  > >  -
>  >  > >    if (http_parser_has_error(parser) ) {
>  >  > >      return -1;
>  >  > >    } else if (http_parser_is_finished(parser) ) {
>  >  > >  @@ -161,5 +149,5 @@ int http_parser_has_error(http_parser *parser) {
>  >  > >   }
>  >  > >
>  >  > >   int http_parser_is_finished(http_parser *parser) {
>  >  > >  -  return parser->cs == http_parser_first_final;
>  >  > >  +  return parser->cs >= http_parser_first_final;
>  >  > >   }
>  >  > >  --
>  >  > >
>  >  > >
>  >  > > Eric Wong
>  >  > --
>  >  > Evan Weaver
>  >  > Cloudburst, LLC
>  >
>  >
>  > _______________________________________________
>  >  Mongrel-development mailing list
>  >  Mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
>  >  http://rubyforge.org/mailman/listinfo/mongrel-development
>  >
>
>
>
>  --
>  Evan Weaver
>  Cloudburst, LLC
>  _______________________________________________
>  Mongrel-development mailing list
>  Mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
>  http://rubyforge.org/mailman/listinfo/mongrel-development
>