From b895d3d0393a647d3602783bc53bf68e223e51c9 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Jun 2010 05:17:14 +0000 Subject: pass-through body.to_path when wrapping the body Certain configurations of Rainbows! (and Zbatery) are able to use the return value of body.to_path to serve static files more efficiently. This also allows middleware like Rack::Contrib::Sendfile to work properly higher up the stack, too. --- ext/clogger_ext/clogger.c | 47 ++++++++++++- ext/clogger_ext/ruby_1_9_compat.h | 37 ++++++++++ lib/clogger.rb | 9 +++ lib/clogger/pure.rb | 24 ++++++- test/test_clogger_to_path.rb | 140 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 test/test_clogger_to_path.rb diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 100392d..8392c4c 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -1,8 +1,14 @@ #define _BSD_SOURCE #include +#ifdef HAVE_RUBY_IO_H +# include +#else +# include +#endif #include #include #include +#include #include #include #include @@ -89,7 +95,10 @@ static ID to_s_id; static ID size_id; static ID sq_brace_id; static ID new_id; +static ID to_path_id; +static ID to_io_id; static VALUE cClogger; +static VALUE cToPath; static VALUE mFormat; static VALUE cHeaderHash; @@ -638,7 +647,7 @@ static VALUE body_iter_i(VALUE str, VALUE memop) return rb_yield(str); } -static VALUE wrap_close(struct clogger *c) +static VALUE body_close(struct clogger *c) { if (rb_respond_to(c->body, close_id)) return rb_funcall(c->body, close_id, 0); @@ -676,7 +685,7 @@ static VALUE clogger_close(VALUE self) { struct clogger *c = clogger_get(self); - return rb_ensure(wrap_close, (VALUE)c, cwrite, (VALUE)c); + return rb_ensure(body_close, (VALUE)c, cwrite, (VALUE)c); } /* :nodoc: */ @@ -748,6 +757,9 @@ static VALUE clogger_call(VALUE self, VALUE env) rv = ccall(c, env); assert(!OBJ_FROZEN(rv) && "frozen response array"); + + if (rb_respond_to(c->body, to_path_id)) + self = rb_funcall(cToPath, new_id, 1, self); rb_ary_store(rv, 2, self); return rv; @@ -778,6 +790,33 @@ static VALUE clogger_init_copy(VALUE clone, VALUE orig) #define CONST_GLOBAL_STR(val) CONST_GLOBAL_STR2(val, #val) +static VALUE to_path(VALUE self) +{ + struct clogger *c = clogger_get(RSTRUCT_PTR(self)[0]); + VALUE path = rb_funcall(c->body, to_path_id, 0); + struct stat sb; + int rv; + const char *cpath; + + Check_Type(path, T_STRING); + cpath = RSTRING_PTR(path); + + /* try to avoid an extra path lookup */ + if (rb_respond_to(c->body, to_io_id)) + rv = fstat(my_fileno(c->body), &sb); + /* + * Rainbows! can use "/dev/fd/%d" in to_path output to avoid + * extra open() syscalls, too. + */ + else if (sscanf(cpath, "/dev/fd/%d", &rv) == 1) + rv = fstat(rv, &sb); + else + rv = stat(cpath, &sb); + + c->body_bytes_sent = rv == 0 ? sb.st_size : 0; + return path; +} + void Init_clogger_ext(void) { VALUE tmp; @@ -791,6 +830,8 @@ void Init_clogger_ext(void) size_id = rb_intern("size"); sq_brace_id = rb_intern("[]"); new_id = rb_intern("new"); + to_path_id = rb_intern("to_path"); + to_io_id = rb_intern("to_io"); cClogger = rb_define_class("Clogger", rb_cObject); mFormat = rb_define_module_under(cClogger, "Format"); rb_define_alloc_func(cClogger, clogger_alloc); @@ -820,4 +861,6 @@ void Init_clogger_ext(void) tmp = rb_const_get(rb_cObject, rb_intern("Rack")); tmp = rb_const_get(tmp, rb_intern("Utils")); cHeaderHash = rb_const_get(tmp, rb_intern("HeaderHash")); + cToPath = rb_const_get(cClogger, rb_intern("ToPath")); + rb_define_method(cToPath, "to_path", to_path, 0); } diff --git a/ext/clogger_ext/ruby_1_9_compat.h b/ext/clogger_ext/ruby_1_9_compat.h index e0ba4ac..b6caa96 100644 --- a/ext/clogger_ext/ruby_1_9_compat.h +++ b/ext/clogger_ext/ruby_1_9_compat.h @@ -11,6 +11,12 @@ #ifndef RARRAY_LEN # define RARRAY_LEN(s) (RARRAY(s)->len) #endif +#ifndef RSTRUCT_PTR +# define RSTRUCT_PTR(s) (RSTRUCT(s)->ptr) +#endif +#ifndef RSTRUCT_LEN +# define RSTRUCT_LEN(s) (RSTRUCT(s)->len) +#endif #ifndef HAVE_RB_STR_SET_LEN /* this is taken from Ruby 1.8.7, 1.8.6 may not have it */ @@ -21,3 +27,34 @@ static void rb_18_str_set_len(VALUE str, long len) } #define rb_str_set_len(str,len) rb_18_str_set_len(str,len) #endif + +#if ! HAVE_RB_IO_T +# define rb_io_t OpenFile +#endif + +#ifdef GetReadFile +# define FPTR_TO_FD(fptr) (fileno(GetReadFile(fptr))) +#else +# if !HAVE_RB_IO_T || (RUBY_VERSION_MAJOR == 1 && RUBY_VERSION_MINOR == 8) +# define FPTR_TO_FD(fptr) fileno(fptr->f) +# else +# define FPTR_TO_FD(fptr) fptr->fd +# endif +#endif + +static int my_fileno(VALUE io) +{ + rb_io_t *fptr; + + for (;;) { + switch (TYPE(io)) { + case T_FILE: { + GetOpenFile(io, fptr); + return FPTR_TO_FD(fptr); + } + default: + io = rb_convert_type(io, T_FILE, "IO", "to_io"); + /* retry */ + } + } +} diff --git a/lib/clogger.rb b/lib/clogger.rb index ba047f5..a827632 100644 --- a/lib/clogger.rb +++ b/lib/clogger.rb @@ -36,6 +36,15 @@ class Clogger :request_uri => 7 } + # proxy class to avoid clobbering the +to_path+ optimization when + # using static files + class ToPath < Struct.new(:clogger) + def each(&block); clogger.each(&block); end + def close; clogger.close; end + + # to_path is defined in Clogger::Pure or the C extension + end + private CGI_ENV = Regexp.new('\A\$(' << diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 50e4f6e..0dd5021 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -5,6 +5,9 @@ # the original C extension code so it's not very Ruby-ish... class Clogger + attr_accessor :env, :status, :headers, :body + attr_writer :body_bytes_sent + def initialize(app, opts = {}) # trigger autoload to avoid thread-safety issues later on Rack::Utils::HeaderHash.new({}) @@ -30,8 +33,13 @@ class Clogger headers = Rack::Utils::HeaderHash.new(headers) if @need_resp if @wrap_body @reentrant = env['rack.multithread'] if @reentrant.nil? - @env, @status, @headers, @body = env, status, headers, body - return [ status, headers, @reentrant ? self.dup : self ] + wbody = @reentrant ? self.dup : self + wbody.env = env + wbody.status = status + wbody.headers = headers + wbody.body = body + wbody = Clogger::ToPath.new(wbody) if body.respond_to?(:to_path) + return [ status, headers, wbody ] end log(env, status, headers) [ status, headers, body ] @@ -139,4 +147,16 @@ private }.join('') end + class ToPath + def to_path + rv = (body = clogger.body).to_path + + # try to avoid unnecessary path lookups with to_io.stat instead of + # File.stat + clogger.body_bytes_sent = + (body.respond_to?(:to_io) ? body.to_io.stat : File.stat(rv)).size + rv + end + end + end diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb new file mode 100644 index 0000000..4da3103 --- /dev/null +++ b/test/test_clogger_to_path.rb @@ -0,0 +1,140 @@ +# -*- encoding: binary -*- +$stderr.sync = $stdout.sync = true +require "test/unit" +require "date" +require "stringio" +require "rack" +require "clogger" + +class MyBody < Struct.new(:to_path, :closed) + def each(&block) + raise RuntimeError, "each should never get called" + end + + def close + self.closed = true + end +end + +class TestCloggerToPath < Test::Unit::TestCase + + def setup + @req = { + "REQUEST_METHOD" => "GET", + "HTTP_VERSION" => "HTTP/1.0", + "HTTP_USER_AGENT" => 'echo and socat \o/', + "PATH_INFO" => "/", + "QUERY_STRING" => "", + "rack.errors" => $stderr, + "rack.input" => File.open('/dev/null', 'rb'), + "REMOTE_ADDR" => '127.0.0.1', + } + end + + def test_wraps_to_path + logger = StringIO.new + tmp = Tempfile.new('') + b = nil + app = Rack::Builder.new do + tmp.syswrite(' ' * 365) + h = { + 'Content-Length' => '0', + 'Content-Type' => 'text/plain', + } + use Clogger, + :logger => logger, + :reentrant => true, + :format => '$body_bytes_sent $status' + run lambda { |env| [ 200, h, b = MyBody.new(tmp.path) ] } + end.to_app + + status, headers, body = app.call(@req) + assert_instance_of(Clogger::ToPath, body) + assert logger.string.empty? + assert_equal tmp.path, body.to_path + body.close + assert b.closed, "close passed through" + assert_equal "365 200\n", logger.string + end + + def test_wraps_to_path_dev_fd + logger = StringIO.new + tmp = Tempfile.new('') + b = nil + app = Rack::Builder.new do + tmp.syswrite(' ' * 365) + h = { + 'Content-Length' => '0', + 'Content-Type' => 'text/plain', + } + use Clogger, + :logger => logger, + :reentrant => true, + :format => '$body_bytes_sent $status' + run lambda { |env| [ 200, h, b = MyBody.new("/dev/fd/#{tmp.fileno}") ] } + end.to_app + + status, headers, body = app.call(@req) + assert_instance_of(Clogger::ToPath, body) + assert logger.string.empty? + assert_equal "/dev/fd/#{tmp.fileno}", body.to_path + body.close + assert b.closed + assert_equal "365 200\n", logger.string + end + + def test_wraps_to_path_to_io + logger = StringIO.new + tmp = Tempfile.new('') + def tmp.to_io + @to_io_called = super + end + def tmp.to_path + path + end + app = Rack::Builder.new do + tmp.syswrite(' ' * 365) + tmp.sysseek(0) + h = { + 'Content-Length' => '0', + 'Content-Type' => 'text/plain', + } + use Clogger, + :logger => logger, + :reentrant => true, + :format => '$body_bytes_sent $status' + run lambda { |env| [ 200, h, tmp ] } + end.to_app + + status, headers, body = app.call(@req) + assert_instance_of(Clogger::ToPath, body) + body.to_path + assert_kind_of IO, tmp.instance_variable_get(:@to_io_called) + assert logger.string.empty? + assert ! tmp.closed? + body.close + assert tmp.closed? + assert_equal "365 200\n", logger.string + end + + def test_does_not_wrap_to_path + logger = StringIO.new + app = Rack::Builder.new do + h = { + 'Content-Length' => '3', + 'Content-Type' => 'text/plain', + } + use Clogger, + :logger => logger, + :reentrant => true, + :format => '$body_bytes_sent $status' + run lambda { |env| [ 200, h, [ "hi\n" ] ] } + end.to_app + status, headers, body = app.call(@req) + assert_instance_of(Clogger, body) + assert logger.string.empty? + body.close + assert ! logger.string.empty? + end + +end -- cgit v1.2.3-24-ge0c7