rack-devel archive mirror (unofficial) https://groups.google.com/group/rack-devel
 help / color / mirror / Atom feed
* [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
@ 2013-10-02 23:10 ruby
  2013-10-02 23:55 ` Konstantin Haase
  0 siblings, 1 reply; 8+ messages in thread
From: ruby @ 2013-10-02 23:10 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

From: Aman Gupta <aman@tmm1.net>

This new `warmup` method takes a block which is invoked after the app
is built. The block can be used to make mock requests that ensure
all application dependencies are loaded before the app starts
serving traffic.

With complex frameworks like Rails, many dependencies are auto-loaded
and data like mime-type and i18n is not loaded into memory by default. This
often means the first few requests handled by an application are quite
slow.

With this patch, config.ru can simply make requests via warmup to
exercise the app before it is used:

  $ tail -4 config.ru
  warmup do |app|
    client = Rack::MockRequest.new(app)
    client.get('/')
  end
---
 lib/rack/builder.rb  | 19 +++++++++++++++++--
 test/spec_builder.rb | 11 +++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
index 66dc7bd..fa3a1ea 100644
--- a/lib/rack/builder.rb
+++ b/lib/rack/builder.rb
@@ -51,7 +51,7 @@ module Rack
     end
 
     def initialize(default_app = nil,&block)
-      @use, @map, @run = [], nil, default_app
+      @use, @map, @run, @warmup = [], nil, default_app, nil
       instance_eval(&block) if block_given?
     end
 
@@ -104,6 +104,19 @@ module Rack
       @run = app
     end
 
+    # Takes a lambda or block that is used to warm-up the application.
+    #
+    #   warmup do |app|
+    #     client = Rack::MockRequest.new(app)
+    #     client.get('/')
+    #   end
+    #
+    #   use SomeMiddleware
+    #   run MyApp
+    def warmup(prc=nil, &block)
+      @warmup = prc || block
+    end
+
     # Creates a route within the application.
     #
     #   Rack::Builder.app do
@@ -131,7 +144,9 @@ module Rack
     def to_app
       app = @map ? generate_map(@run, @map) : @run
       fail "missing run or map statement" unless app
-      @use.reverse.inject(app) { |a,e| e[a] }
+      app = @use.reverse.inject(app) { |a,e| e[a] }
+      @warmup.call(app) if @warmup
+      app
     end
 
     def call(env)
diff --git a/test/spec_builder.rb b/test/spec_builder.rb
index 0774f59..20ea668 100644
--- a/test/spec_builder.rb
+++ b/test/spec_builder.rb
@@ -130,6 +130,17 @@ describe Rack::Builder do
     Rack::MockRequest.new(app).get("/foo").should.be.server_error
   end
 
+  it "yields the generated app to a block for warmup" do
+    warmed_up_app = nil
+
+    app = Rack::Builder.new do
+      warmup { |a| warmed_up_app = a }
+      run lambda { |env| [200, {}, []] }
+    end.to_app
+
+    warmed_up_app.should.equal app
+  end
+
   should "initialize apps once" do
     app = builder do
       class AppClass
-- 
1.8.3.4

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

* Re: [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
  2013-10-02 23:10 [RFC/PATCH] Add Rack::Builder#warmup method for app preloading ruby
@ 2013-10-02 23:55 ` Konstantin Haase
  2013-10-29  0:02   ` James Tucker
  0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Haase @ 2013-10-02 23:55 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

"Could you rebase and add some tests." /trololo

I generally like the idea. I think we might have to remove Builder#call, though, just to avoid concurrency issues and double firing warmup blocks.

Konstantin

On Oct 3, 2013, at 1:10 AM, ruby@tmm1.net wrote:

> From: Aman Gupta <aman@tmm1.net>
> 
> This new `warmup` method takes a block which is invoked after the app
> is built. The block can be used to make mock requests that ensure
> all application dependencies are loaded before the app starts
> serving traffic.
> 
> With complex frameworks like Rails, many dependencies are auto-loaded
> and data like mime-type and i18n is not loaded into memory by default. This
> often means the first few requests handled by an application are quite
> slow.
> 
> With this patch, config.ru can simply make requests via warmup to
> exercise the app before it is used:
> 
>  $ tail -4 config.ru
>  warmup do |app|
>    client = Rack::MockRequest.new(app)
>    client.get('/')
>  end
> ---
> lib/rack/builder.rb  | 19 +++++++++++++++++--
> test/spec_builder.rb | 11 +++++++++++
> 2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
> index 66dc7bd..fa3a1ea 100644
> --- a/lib/rack/builder.rb
> +++ b/lib/rack/builder.rb
> @@ -51,7 +51,7 @@ module Rack
>     end
> 
>     def initialize(default_app = nil,&block)
> -      @use, @map, @run = [], nil, default_app
> +      @use, @map, @run, @warmup = [], nil, default_app, nil
>       instance_eval(&block) if block_given?
>     end
> 
> @@ -104,6 +104,19 @@ module Rack
>       @run = app
>     end
> 
> +    # Takes a lambda or block that is used to warm-up the application.
> +    #
> +    #   warmup do |app|
> +    #     client = Rack::MockRequest.new(app)
> +    #     client.get('/')
> +    #   end
> +    #
> +    #   use SomeMiddleware
> +    #   run MyApp
> +    def warmup(prc=nil, &block)
> +      @warmup = prc || block
> +    end
> +
>     # Creates a route within the application.
>     #
>     #   Rack::Builder.app do
> @@ -131,7 +144,9 @@ module Rack
>     def to_app
>       app = @map ? generate_map(@run, @map) : @run
>       fail "missing run or map statement" unless app
> -      @use.reverse.inject(app) { |a,e| e[a] }
> +      app = @use.reverse.inject(app) { |a,e| e[a] }
> +      @warmup.call(app) if @warmup
> +      app
>     end
> 
>     def call(env)
> diff --git a/test/spec_builder.rb b/test/spec_builder.rb
> index 0774f59..20ea668 100644
> --- a/test/spec_builder.rb
> +++ b/test/spec_builder.rb
> @@ -130,6 +130,17 @@ describe Rack::Builder do
>     Rack::MockRequest.new(app).get("/foo").should.be.server_error
>   end
> 
> +  it "yields the generated app to a block for warmup" do
> +    warmed_up_app = nil
> +
> +    app = Rack::Builder.new do
> +      warmup { |a| warmed_up_app = a }
> +      run lambda { |env| [200, {}, []] }
> +    end.to_app
> +
> +    warmed_up_app.should.equal app
> +  end
> +
>   should "initialize apps once" do
>     app = builder do
>       class AppClass
> -- 
> 1.8.3.4
> 
> -- 
> 
> --- 
> You received this message because you are subscribed to the Google Groups "Rack Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

* Re: [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
  2013-10-02 23:55 ` Konstantin Haase
@ 2013-10-29  0:02   ` James Tucker
  2013-10-29  0:21     ` Santiago Pastorino
  2013-10-29 17:54     ` Eric Wong
  0 siblings, 2 replies; 8+ messages in thread
From: James Tucker @ 2013-10-29  0:02 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

[-- Attachment #1: Type: text/plain, Size: 4480 bytes --]

Yeah, Builder#call leads to mistakes all the time and then people claiming
performance sucks. I don't think the simple use case there ever helped.

I'll keep this starred for my next round of merges. Probably not before 1.6.


On Wed, Oct 2, 2013 at 4:55 PM, Konstantin Haase <k.haase@finn.de> wrote:

> "Could you rebase and add some tests." /trololo
>
> I generally like the idea. I think we might have to remove Builder#call,
> though, just to avoid concurrency issues and double firing warmup blocks.
>
> Konstantin
>
> On Oct 3, 2013, at 1:10 AM, ruby@tmm1.net wrote:
>
> > From: Aman Gupta <aman@tmm1.net>
> >
> > This new `warmup` method takes a block which is invoked after the app
> > is built. The block can be used to make mock requests that ensure
> > all application dependencies are loaded before the app starts
> > serving traffic.
> >
> > With complex frameworks like Rails, many dependencies are auto-loaded
> > and data like mime-type and i18n is not loaded into memory by default.
> This
> > often means the first few requests handled by an application are quite
> > slow.
> >
> > With this patch, config.ru can simply make requests via warmup to
> > exercise the app before it is used:
> >
> >  $ tail -4 config.ru
> >  warmup do |app|
> >    client = Rack::MockRequest.new(app)
> >    client.get('/')
> >  end
> > ---
> > lib/rack/builder.rb  | 19 +++++++++++++++++--
> > test/spec_builder.rb | 11 +++++++++++
> > 2 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
> > index 66dc7bd..fa3a1ea 100644
> > --- a/lib/rack/builder.rb
> > +++ b/lib/rack/builder.rb
> > @@ -51,7 +51,7 @@ module Rack
> >     end
> >
> >     def initialize(default_app = nil,&block)
> > -      @use, @map, @run = [], nil, default_app
> > +      @use, @map, @run, @warmup = [], nil, default_app, nil
> >       instance_eval(&block) if block_given?
> >     end
> >
> > @@ -104,6 +104,19 @@ module Rack
> >       @run = app
> >     end
> >
> > +    # Takes a lambda or block that is used to warm-up the application.
> > +    #
> > +    #   warmup do |app|
> > +    #     client = Rack::MockRequest.new(app)
> > +    #     client.get('/')
> > +    #   end
> > +    #
> > +    #   use SomeMiddleware
> > +    #   run MyApp
> > +    def warmup(prc=nil, &block)
> > +      @warmup = prc || block
> > +    end
> > +
> >     # Creates a route within the application.
> >     #
> >     #   Rack::Builder.app do
> > @@ -131,7 +144,9 @@ module Rack
> >     def to_app
> >       app = @map ? generate_map(@run, @map) : @run
> >       fail "missing run or map statement" unless app
> > -      @use.reverse.inject(app) { |a,e| e[a] }
> > +      app = @use.reverse.inject(app) { |a,e| e[a] }
> > +      @warmup.call(app) if @warmup
> > +      app
> >     end
> >
> >     def call(env)
> > diff --git a/test/spec_builder.rb b/test/spec_builder.rb
> > index 0774f59..20ea668 100644
> > --- a/test/spec_builder.rb
> > +++ b/test/spec_builder.rb
> > @@ -130,6 +130,17 @@ describe Rack::Builder do
> >     Rack::MockRequest.new(app).get("/foo").should.be.server_error
> >   end
> >
> > +  it "yields the generated app to a block for warmup" do
> > +    warmed_up_app = nil
> > +
> > +    app = Rack::Builder.new do
> > +      warmup { |a| warmed_up_app = a }
> > +      run lambda { |env| [200, {}, []] }
> > +    end.to_app
> > +
> > +    warmed_up_app.should.equal app
> > +  end
> > +
> >   should "initialize apps once" do
> >     app = builder do
> >       class AppClass
> > --
> > 1.8.3.4
> >
> > --
> >
> > ---
> > You received this message because you are subscribed to the Google
> Groups "Rack Development" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to rack-devel+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/groups/opt_out.
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Rack Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rack-devel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[-- Attachment #2: Type: text/html, Size: 6212 bytes --]

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

* Re: [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
  2013-10-29  0:02   ` James Tucker
@ 2013-10-29  0:21     ` Santiago Pastorino
  2013-10-29 17:54     ` Eric Wong
  1 sibling, 0 replies; 8+ messages in thread
From: Santiago Pastorino @ 2013-10-29  0:21 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

[-- Attachment #1: Type: text/plain, Size: 5052 bytes --]

James, I already merged this in master.


On Mon, Oct 28, 2013 at 10:02 PM, James Tucker <jftucker@gmail.com> wrote:

> Yeah, Builder#call leads to mistakes all the time and then people claiming
> performance sucks. I don't think the simple use case there ever helped.
>
> I'll keep this starred for my next round of merges. Probably not before
> 1.6.
>
>
> On Wed, Oct 2, 2013 at 4:55 PM, Konstantin Haase <k.haase@finn.de> wrote:
>
>> "Could you rebase and add some tests." /trololo
>>
>> I generally like the idea. I think we might have to remove Builder#call,
>> though, just to avoid concurrency issues and double firing warmup blocks.
>>
>> Konstantin
>>
>> On Oct 3, 2013, at 1:10 AM, ruby@tmm1.net wrote:
>>
>> > From: Aman Gupta <aman@tmm1.net>
>> >
>> > This new `warmup` method takes a block which is invoked after the app
>> > is built. The block can be used to make mock requests that ensure
>> > all application dependencies are loaded before the app starts
>> > serving traffic.
>> >
>> > With complex frameworks like Rails, many dependencies are auto-loaded
>> > and data like mime-type and i18n is not loaded into memory by default.
>> This
>> > often means the first few requests handled by an application are quite
>> > slow.
>> >
>> > With this patch, config.ru can simply make requests via warmup to
>> > exercise the app before it is used:
>> >
>> >  $ tail -4 config.ru
>> >  warmup do |app|
>> >    client = Rack::MockRequest.new(app)
>> >    client.get('/')
>> >  end
>> > ---
>> > lib/rack/builder.rb  | 19 +++++++++++++++++--
>> > test/spec_builder.rb | 11 +++++++++++
>> > 2 files changed, 28 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
>> > index 66dc7bd..fa3a1ea 100644
>> > --- a/lib/rack/builder.rb
>> > +++ b/lib/rack/builder.rb
>> > @@ -51,7 +51,7 @@ module Rack
>> >     end
>> >
>> >     def initialize(default_app = nil,&block)
>> > -      @use, @map, @run = [], nil, default_app
>> > +      @use, @map, @run, @warmup = [], nil, default_app, nil
>> >       instance_eval(&block) if block_given?
>> >     end
>> >
>> > @@ -104,6 +104,19 @@ module Rack
>> >       @run = app
>> >     end
>> >
>> > +    # Takes a lambda or block that is used to warm-up the application.
>> > +    #
>> > +    #   warmup do |app|
>> > +    #     client = Rack::MockRequest.new(app)
>> > +    #     client.get('/')
>> > +    #   end
>> > +    #
>> > +    #   use SomeMiddleware
>> > +    #   run MyApp
>> > +    def warmup(prc=nil, &block)
>> > +      @warmup = prc || block
>> > +    end
>> > +
>> >     # Creates a route within the application.
>> >     #
>> >     #   Rack::Builder.app do
>> > @@ -131,7 +144,9 @@ module Rack
>> >     def to_app
>> >       app = @map ? generate_map(@run, @map) : @run
>> >       fail "missing run or map statement" unless app
>> > -      @use.reverse.inject(app) { |a,e| e[a] }
>> > +      app = @use.reverse.inject(app) { |a,e| e[a] }
>> > +      @warmup.call(app) if @warmup
>> > +      app
>> >     end
>> >
>> >     def call(env)
>> > diff --git a/test/spec_builder.rb b/test/spec_builder.rb
>> > index 0774f59..20ea668 100644
>> > --- a/test/spec_builder.rb
>> > +++ b/test/spec_builder.rb
>> > @@ -130,6 +130,17 @@ describe Rack::Builder do
>> >     Rack::MockRequest.new(app).get("/foo").should.be.server_error
>> >   end
>> >
>> > +  it "yields the generated app to a block for warmup" do
>> > +    warmed_up_app = nil
>> > +
>> > +    app = Rack::Builder.new do
>> > +      warmup { |a| warmed_up_app = a }
>> > +      run lambda { |env| [200, {}, []] }
>> > +    end.to_app
>> > +
>> > +    warmed_up_app.should.equal app
>> > +  end
>> > +
>> >   should "initialize apps once" do
>> >     app = builder do
>> >       class AppClass
>> > --
>> > 1.8.3.4
>> >
>> > --
>> >
>> > ---
>> > You received this message because you are subscribed to the Google
>> Groups "Rack Development" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to rack-devel+unsubscribe@googlegroups.com.
>> > For more options, visit https://groups.google.com/groups/opt_out.
>>
>> --
>>
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "Rack Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to rack-devel+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>
>  --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Rack Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rack-devel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[-- Attachment #2: Type: text/html, Size: 7213 bytes --]

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

* Re: [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
  2013-10-29  0:02   ` James Tucker
  2013-10-29  0:21     ` Santiago Pastorino
@ 2013-10-29 17:54     ` Eric Wong
  2013-11-06  2:57       ` Eric Wong
  1 sibling, 1 reply; 8+ messages in thread
From: Eric Wong @ 2013-10-29 17:54 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

James Tucker <jftucker@gmail.com> wrote:
> Yeah, Builder#call leads to mistakes all the time and then people claiming
> performance sucks. I don't think the simple use case there ever helped.

Just curious, is this in test suites or code running on live servers?
I can't imagine there are many one-off webservers existing and using
that directly...

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

* Re: [RFC/PATCH] Add Rack::Builder#warmup method for app preloading.
  2013-10-29 17:54     ` Eric Wong
@ 2013-11-06  2:57       ` Eric Wong
  2013-11-06  3:12         ` [PATCH] builder: avoid to_app on every request when using map Eric Wong
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Wong @ 2013-11-06  2:57 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

Eric Wong <normalperson@yhbt.net> wrote:
> James Tucker <jftucker@gmail.com> wrote:
> > Yeah, Builder#call leads to mistakes all the time and then people claiming
> > performance sucks. I don't think the simple use case there ever helped.
> 
> Just curious, is this in test suites or code running on live servers?
> I can't imagine there are many one-off webservers existing and using
> that directly...

Answering my own question, the "map" directive in Builder seems to
trigger Builder#to_app in every single request.

It looks like map completely defeats warmup and preloading...

-------------------------------8<-----------------------------
require 'rack/lobster'
class Wtf
  def initialize(app)
    # this prints every single request :<
    p "#$$ #{Time.now} initialized"
    @app = app
  end
  def call env
    @app.call(env)
  end
end

map "http://example.com/" do
  use Wtf
  run Rack::Lobster.new
end
-------------------------------8<-----------------------------
(tested with "rackup -s webrick", too, so none of my own broken server
 code running)

But yeah, extensive use of map on *.bogomips.org and *.yhbt.net
now that I've switched that hole thing over to yahns...

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

* [PATCH] builder: avoid to_app on every request when using map
  2013-11-06  2:57       ` Eric Wong
@ 2013-11-06  3:12         ` Eric Wong
  2014-07-15  4:09           ` James Tucker
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Wong @ 2013-11-06  3:12 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

Eric Wong <normalperson@yhbt.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> > James Tucker <jftucker@gmail.com> wrote:
> > > Yeah, Builder#call leads to mistakes all the time and then people claiming
> > > performance sucks. I don't think the simple use case there ever helped.
> > 
> > Just curious, is this in test suites or code running on live servers?
> > I can't imagine there are many one-off webservers existing and using
> > that directly...
> 
> Answering my own question, the "map" directive in Builder seems to
> trigger Builder#to_app in every single request.
> 
> It looks like map completely defeats warmup and preloading...

OK, I can't tell if my this has any bad side effects (specs pass!),
but I'm already running this on yhbt.net:

From: Eric Wong <normalperson@yhbt.net>
Date: Wed, 6 Nov 2013 03:02:48 +0000
Subject: [PATCH] builder: avoid to_app on every request when using map

By calling to_app immediately after initializing the per-map
Rack::Builder instances.  Otherwise, benefits of warmup and web
servers taking advantage of CoW are lost.

This passes tests, and is lightly tested and I have not verified
this for any negative consequences or incompatibilities.
---
 lib/rack/builder.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
index fa3a1ea..2aa37b6 100644
--- a/lib/rack/builder.rb
+++ b/lib/rack/builder.rb
@@ -157,7 +157,7 @@ module Rack
 
     def generate_map(default_app, mapping)
       mapped = default_app ? {'/' => default_app} : {}
-      mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b) }
+      mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b).to_app }
       URLMap.new(mapped)
     end
   end
-- 
The following changes since commit df1506b0825a096514fcb3821563bf9e8fd52743:

  Merge pull request #617 from tmm1/builder-warmup (2013-10-25 21:10:40 -0700)

are available in the git repository at:

  git://git.bogomips.org/rack.git builder-map-to_app

for you to fetch changes up to fd023bafed0ad97e08d15381f35600e3a5b32808:

  builder: avoid to_app on every request when using map (2013-11-06 03:08:38 +0000)

----------------------------------------------------------------
Eric Wong (1):
      builder: avoid to_app on every request when using map

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

* Re: [PATCH] builder: avoid to_app on every request when using map
  2013-11-06  3:12         ` [PATCH] builder: avoid to_app on every request when using map Eric Wong
@ 2014-07-15  4:09           ` James Tucker
  0 siblings, 0 replies; 8+ messages in thread
From: James Tucker @ 2014-07-15  4:09 UTC (permalink / raw)
  To: rack-devel; +Cc: Aman Gupta

[-- Attachment #1: Type: text/plain, Size: 3118 bytes --]

Thanks!


On Tue, Nov 5, 2013 at 7:12 PM, Eric Wong <normalperson@yhbt.net> wrote:

> Eric Wong <normalperson@yhbt.net> wrote:
> > Eric Wong <normalperson@yhbt.net> wrote:
> > > James Tucker <jftucker@gmail.com> wrote:
> > > > Yeah, Builder#call leads to mistakes all the time and then people
> claiming
> > > > performance sucks. I don't think the simple use case there ever
> helped.
> > >
> > > Just curious, is this in test suites or code running on live servers?
> > > I can't imagine there are many one-off webservers existing and using
> > > that directly...
> >
> > Answering my own question, the "map" directive in Builder seems to
> > trigger Builder#to_app in every single request.
> >
> > It looks like map completely defeats warmup and preloading...
>
> OK, I can't tell if my this has any bad side effects (specs pass!),
> but I'm already running this on yhbt.net:
>
> From: Eric Wong <normalperson@yhbt.net>
> Date: Wed, 6 Nov 2013 03:02:48 +0000
> Subject: [PATCH] builder: avoid to_app on every request when using map
>
> By calling to_app immediately after initializing the per-map
> Rack::Builder instances.  Otherwise, benefits of warmup and web
> servers taking advantage of CoW are lost.
>
> This passes tests, and is lightly tested and I have not verified
> this for any negative consequences or incompatibilities.
> ---
>  lib/rack/builder.rb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
> index fa3a1ea..2aa37b6 100644
> --- a/lib/rack/builder.rb
> +++ b/lib/rack/builder.rb
> @@ -157,7 +157,7 @@ module Rack
>
>      def generate_map(default_app, mapping)
>        mapped = default_app ? {'/' => default_app} : {}
> -      mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b) }
> +      mapping.each { |r,b| mapped[r] = self.class.new(default_app,
> &b).to_app }
>        URLMap.new(mapped)
>      end
>    end
> --
> The following changes since commit
> df1506b0825a096514fcb3821563bf9e8fd52743:
>
>   Merge pull request #617 from tmm1/builder-warmup (2013-10-25 21:10:40
> -0700)
>
> are available in the git repository at:
>
>   git://git.bogomips.org/rack.git builder-map-to_app
>
> for you to fetch changes up to fd023bafed0ad97e08d15381f35600e3a5b32808:
>
>   builder: avoid to_app on every request when using map (2013-11-06
> 03:08:38 +0000)
>
> ----------------------------------------------------------------
> Eric Wong (1):
>       builder: avoid to_app on every request when using map
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Rack Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rack-devel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: text/html, Size: 4438 bytes --]

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

end of thread, other threads:[~2014-07-15  4:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-02 23:10 [RFC/PATCH] Add Rack::Builder#warmup method for app preloading ruby
2013-10-02 23:55 ` Konstantin Haase
2013-10-29  0:02   ` James Tucker
2013-10-29  0:21     ` Santiago Pastorino
2013-10-29 17:54     ` Eric Wong
2013-11-06  2:57       ` Eric Wong
2013-11-06  3:12         ` [PATCH] builder: avoid to_app on every request when using map Eric Wong
2014-07-15  4:09           ` James Tucker

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