mirror of mongrel-development@rubyforge.org (inactive)
 help / color / mirror / Atom feed
From: Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org>
To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
Subject: Re: [PATCH] http11: ~6% performance increase in header parsing
Date: Mon, 24 Mar 2008 20:43:07 -0700	[thread overview]
Message-ID: <20080325034307.GA13379@soma> (raw)
In-Reply-To: <b6f68fc60803241139g70b2d8a3v5d01e7472367fb74-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Evan Weaver <evan-72XWLPH10WVXUHR/Jj/Uug@public.gmane.org> wrote:
> Hi all,
> 
> I backported the trunk parser changes and applied the Ragel fix to
> branches/stable_1-2 . Two tests are currently failing... could someone
> (Eric, Ry?) take a look at these? I am not good enough with Ragel to
> be able to say whether or not the eof change is related to the test
> failures.

Hi Evan,

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 :)

Reverting my attempt at upgrading to Ragel 6 fixes one test...



As for the other test that fails, you uncommented the nasty_pound_header
test in the commit for multi-line.  This test doesn't work in
stable-1_1 nor trunk, and I've never made any effort into getting it
working (nor was it my fault it's broken in the first place :)

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.

The Mongrel HTTP parser code does not appear to handle multi-line
HTTP headers at all, so I'm not surprised that test fails.  I would
suggest disabling the nasty_pound_header test again until we can
successfully parse multi-line headers.

> Thanks,
> 
> 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

  parent reply	other threads:[~2008-03-25  3:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080229015309.GA9080@untitled>
     [not found] ` <20080303044659.5a550c19.zedshaw@zedshaw.com>
     [not found]   ` <20080302123712.GA13979@hand.yhbt.net>
     [not found]     ` <20080306075421.GA1583@hand.yhbt.net>
     [not found]       ` <3ae7f4480803060153v18e955a5j9038f93e558f81d1@mail.gmail.com>
     [not found]         ` <3ae7f4480803060153v18e955a5j9038f93e558f81d1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-03-08  8:12           ` Eric Wong
     [not found]             ` <20080308081210.GA30702-r0bfCMRs158eIZ0/mPfg9Q@public.gmane.org>
2008-03-08  8:14               ` Evan Weaver
     [not found]                 ` <b6f68fc60803080014x3241d9b0n6430c5ec93f7ea2d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-03-08 22:02                   ` Eric Wong
2008-03-24 18:39                     ` Evan Weaver
     [not found]                       ` <b6f68fc60803241139g70b2d8a3v5d01e7472367fb74-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-03-25  3:43                         ` Eric Wong [this message]
2008-03-25  4:53                           ` Evan Weaver
     [not found]                             ` <b6f68fc60803242153i65dbb67re6bc9457060ee03f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-03-25  8:13                               ` ry dahl
2008-03-08 12:37               ` Luis Lavena
2008-03-08 17:37               ` ry dahl
replies disabled, historical list

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).