xml-axkit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Sergeant <m...@sergeant.org>
Subject Two patches that may break things...
Date Tue, 18 May 2004 20:05:25 GMT
I'm looking for committer votes on the two patches below.

I wanted to test whether:

   return OK;

would be faster than what we currently use, which is:

   return DECLINED;

While debugging this I discovered that we try and send the cache twice.  
That is, $Cache->deliver gets called twice in a cached request. At  
least I think it does - I've changed things so many times now I can't  
quite recall :-)

The upshot is that instead of dropping through deep into  
deliver_to_browser, we really should be skipping deliver_to_browser for  
cached delivery.

So I added the following to deliver_to_browser (it made for cleaner  
code to add it there):

diff -u -r1.53 AxKit.pm
--- lib/AxKit.pm        18 Sep 2003 22:12:33 -0000      1.53
+++ lib/AxKit.pm        18 May 2004 18:45:20 -0000
@@ -917,6 +917,9 @@

      AxKit::Debug(4, "delivering to browser");

+    return $result_code if $result_code == DECLINED;
+    return OK if $result_code == DONE;
      if (length($r->pnotes('xml_string'))) {
          # ok, data is in xml_string
          AxKit::Debug(4, "Delivering xml_string");

Now I'm slightly worried that this will break something. However only  
slightly as it all made sense to me.

That was patch #1. Can I get votes on applying this please?


Now I could make send_fd work (it didn't work before for various  

So here's the second patch to do that:

diff -u -b -r1.12 Cache.pm
--- lib/Apache/AxKit/Cache.pm   3 Oct 2003 18:38:38 -0000       1.12
+++ lib/Apache/AxKit/Cache.pm   18 May 2004 19:57:54 -0000
@@ -4,7 +4,7 @@
  use strict;

  use Apache;
-use Apache::Constants qw(OK DECLINED SERVER_ERROR);
+use Apache::Constants qw(OK DECLINED SERVER_ERROR DONE);
  use Apache::AxKit::Exception;
  use Digest::MD5 ();
  use Compress::Zlib qw(gzopen);
@@ -156,10 +156,12 @@

  sub get_fh {
-    my $self = shift;
+    my ($self, $gzip) = @_;
      return if $self->{no_cache};
      my $fh = Apache->gensym();
-    if (AxKit::sysopen($fh, $self->{file}, O_RDONLY, 'raw')) {
+    my $file = $self->{file} . ($gzip ? '.gz' : '');
+    AxKit::Debug(3, "Cache: get_fh opening: $file\n");
+    if (AxKit::sysopen($fh, $file, O_RDONLY, 'raw')) {
          flock($fh, LOCK_SH);
          return $fh;
@@ -223,7 +225,6 @@

      my ($transformer, $doit) = AxKit::get_output_transformer();

      if ($doit) {
@@ -240,13 +241,17 @@

          if ($self->{gzip} && $AxKit::Cfg->DoGzip) {
              AxKit::Debug(4, 'Cache: Delivering gzipped output');
-            $r->filename($self->{file}.'.gz');
+            my $fh = $self->get_fh(1);
+            $r->send_fd($fh);
          else {
-            $r->filename($self->{file});
+            my $fh = $self->get_fh();
+            AxKit::Debug(3, "Cache: send_fd($fh)\n");
+            my $bytes_sent = $r->send_fd($fh);
+            AxKit::Debug(3, "Cache: sent $bytes_sent bytes\n");

-        return DECLINED;
+        return DONE;


I have to return DONE instead of DECLINED so that deliver_to_browser  
translates that into OK. This is the only really messy bit of this  
patch set - how do you bypass deliver_to_browser but still get OK sent  
back to apache? So I had to use this special return code. This is kinda  
ugly, and if anyone has a better idea I'd be glad to see it.

The upside of this patch is it seems to be faster than dropping back to  
apache and letting the next stage deliver the cached file. These are  
the simple benchmark results I got (delivering a very small cached  

   Current CVS: 95 req/s
   Patch #1   : 98 req/s
   Patch 1+2 : 113 req/s

Clearly this is faster. Good. So what's the downside?

Well I can imagine this method doesn't set all the right headers. I  
need to look into this more - am I now missing things like Etags and  
other stuff I might like to have in there? Then again, maybe we could  
still get a speedup by creating those headers myself.

Can anyone else see other downsides?

Vote now!

View raw message