mirror of mongrel-development@rubyforge.org (inactive)
 help / color / mirror / Atom feed
* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]         ` <3ae7f4480803060153v18e955a5j9038f93e558f81d1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-03-08  8:12           ` Eric Wong
       [not found]             ` <20080308081210.GA30702-r0bfCMRs158eIZ0/mPfg9Q@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Wong @ 2008-03-08  8:12 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

ry dahl <ry-Xek56AhD01PHviPkdFu9cA@public.gmane.org> wrote:
> Hi Eric,
> 
> >  I'm getting an even bigger (~22%) performance
> > improvement by predefining
> >  some common HTTP headers as global frozen
> > strings upfront (r992)
> 
> Why don't you do this in Ragel? It will be faster and you don't need
> to depend on bsearch. I pull out content-length header in ragel in
> ebb:
>  http://github.com/ry/ebb/tree/master/src/parser.rl
> (although there is a bug with this because (content_length |
> message_header ) needs some priorities set so message_header isn't
> matching content-length too. should be
>    (content_length >2 | message_header >1 )
> or something).

Then I'd have to define a new C function for every header I wanted to
optimize for, and then also point to that function inside the Ragel file
for each header.

Unless we use something like ERB to generate this code for both Ragel
and C, I'm not sure it's worth the effort to go through with all the
extra code.

Currently, with my C/CPP code, I can add or remove headers to memoize
strings for by adding or removing one line per header in the C file.
Pretty much as easy as it gets maintenance-wise.

> Also - the state machine should be upgraded to compile with ragel 6.
> this basically involves removing %write eof;

Yes.  At the same time, I'm not sure if I should force every other
developer to upgrade... Evan?  Zed?

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [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 12:37               ` Luis Lavena
  2008-03-08 17:37               ` ry dahl
  2 siblings, 1 reply; 9+ messages in thread
From: Evan Weaver @ 2008-03-08  8:14 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

Upgrading ragel is fine with me.

Evan

On Sat, Mar 8, 2008 at 3:12 AM, Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org> wrote:
> ry dahl <ry-Xek56AhD01PHviPkdFu9cA@public.gmane.org> wrote:
>
> > Hi Eric,
>  >
>  > >  I'm getting an even bigger (~22%) performance
>  > > improvement by predefining
>  > >  some common HTTP headers as global frozen
>  > > strings upfront (r992)
>  >
>  > Why don't you do this in Ragel? It will be faster and you don't need
>  > to depend on bsearch. I pull out content-length header in ragel in
>  > ebb:
>  >  http://github.com/ry/ebb/tree/master/src/parser.rl
>  > (although there is a bug with this because (content_length |
>  > message_header ) needs some priorities set so message_header isn't
>  > matching content-length too. should be
>  >    (content_length >2 | message_header >1 )
>  > or something).
>
>  Then I'd have to define a new C function for every header I wanted to
>  optimize for, and then also point to that function inside the Ragel file
>  for each header.
>
>  Unless we use something like ERB to generate this code for both Ragel
>  and C, I'm not sure it's worth the effort to go through with all the
>  extra code.
>
>  Currently, with my C/CPP code, I can add or remove headers to memoize
>  strings for by adding or removing one line per header in the C file.
>  Pretty much as easy as it gets maintenance-wise.
>
>
>  > Also - the state machine should be upgraded to compile with ragel 6.
>  > this basically involves removing %write eof;
>
>  Yes.  At the same time, I'm not sure if I should force every other
>  developer to upgrade... Evan?  Zed?
>
>  --
>  Eric Wong
>
>
> _______________________________________________
>  Mongrel-development mailing list
>  Mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
>  http://rubyforge.org/mailman/listinfo/mongrel-development
>



-- 
Evan Weaver
Cloudburst, LLC

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]             ` <20080308081210.GA30702-r0bfCMRs158eIZ0/mPfg9Q@public.gmane.org>
  2008-03-08  8:14               ` Evan Weaver
@ 2008-03-08 12:37               ` Luis Lavena
  2008-03-08 17:37               ` ry dahl
  2 siblings, 0 replies; 9+ messages in thread
From: Luis Lavena @ 2008-03-08 12:37 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

On Sat, Mar 8, 2008 at 6:12 AM, Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org> wrote:
>
>  Unless we use something like ERB to generate this code for both Ragel
>  and C, I'm not sure it's worth the effort to go through with all the
>  extra code.
>

I think is not worth the effort, and also add complexity to the code
generation, which should be the simpler as possible.

>  Yes.  At the same time, I'm not sure if I should force every other
>  developer to upgrade... Evan?  Zed?

Well, the Ragel compilation is not run every time, but only when the
parser get updated, so I can say will be fine the most common cases
(developers playing with mongrel code already handle the ragel
compiled code).

-- 
Luis Lavena
Multimedia systems
-
A common mistake that people make when trying to design
something completely foolproof is to underestimate
the ingenuity of complete fools.
Douglas Adams

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]             ` <20080308081210.GA30702-r0bfCMRs158eIZ0/mPfg9Q@public.gmane.org>
  2008-03-08  8:14               ` Evan Weaver
  2008-03-08 12:37               ` Luis Lavena
@ 2008-03-08 17:37               ` ry dahl
  2 siblings, 0 replies; 9+ messages in thread
From: ry dahl @ 2008-03-08 17:37 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

>  Then I'd have to define a new C function for every header I wanted to
>  optimize for, and then also point to that function inside the Ragel file
>  for each header.

Nah, you can just define

enum header_types {
 MONGREL_CONTENT_LENGTH,
 MONGREL_CONTENT_TYPE,
 etc
}

and define a single callback to handle them

void (*field_cb)(http_parser*, int header_type, char *at, int len)

ry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]                 ` <b6f68fc60803080014x3241d9b0n6430c5ec93f7ea2d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-03-08 22:02                   ` Eric Wong
  2008-03-24 18:39                     ` Evan Weaver
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Wong @ 2008-03-08 22:02 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
  2008-03-08 22:02                   ` Eric Wong
@ 2008-03-24 18:39                     ` Evan Weaver
       [not found]                       ` <b6f68fc60803241139g70b2d8a3v5d01e7472367fb74-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Evan Weaver @ 2008-03-24 18:39 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

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.

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
>  _______________________________________________
>  Mongrel-development mailing list
>  Mongrel-development-GrnCvJ7WPxnNLxjTenLetw@public.gmane.org
>  http://rubyforge.org/mailman/listinfo/mongrel-development
>



-- 
Evan Weaver
Cloudburst, LLC

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]                       ` <b6f68fc60803241139g70b2d8a3v5d01e7472367fb74-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-03-25  3:43                         ` Eric Wong
  2008-03-25  4:53                           ` Evan Weaver
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Wong @ 2008-03-25  3:43 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
  2008-03-25  3:43                         ` Eric Wong
@ 2008-03-25  4:53                           ` Evan Weaver
       [not found]                             ` <b6f68fc60803242153i65dbb67re6bc9457060ee03f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Evan Weaver @ 2008-03-25  4:53 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

>  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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] http11: ~6% performance increase in header parsing
       [not found]                             ` <b6f68fc60803242153i65dbb67re6bc9457060ee03f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-03-25  8:13                               ` ry dahl
  0 siblings, 0 replies; 9+ messages in thread
From: ry dahl @ 2008-03-25  8:13 UTC (permalink / raw)
  To: mongrel-development-GrnCvJ7WPxnNLxjTenLetw

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-03-25  8:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [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           ` [PATCH] http11: ~6% performance increase in header parsing 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
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

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