Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[Paul Lindner <li...@inuus.com>]

How many people have implemented custom rewriters?  I'm guessing it's a
small number, in which case I'd suggest a more radical reorganization of the
code.

In any case these are all good ideas.

On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com> wrote:

> Hi all,
>
> I'm looking into the rewriting pipeline, specifically for HTTP responses,
> with a few goals in mind:
> 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
> one-off rewriter injection.
>  - And to simplify the rewriter interfaces overall, to allow for uniform
> rewriter profiling code etc.
> 2. Expand RequestRewriters' capability to modify HTTP response headers.
> 3. Maintain - and expand - our ability to cache the result of rewriting
> passes intelligently. Especially important for ImageRewriter, the output of
> which is presently cached for notable performance gains.
>
> To this end, I've created the following CL as a starting point. It builds
> upon the CL I recently submitted adding the ability to manipulate
> MutableContent by byte-array: http://codereview.appspot.com/1032042/show
>
> Downstream of this CL, I'm proposing to change the rewriter interface
> roughly as follows:
>
> OLD
> // Return true if content modified.
> boolean RequestRewriter.rewrite(HttpRequest, HttpResponse, MutableContent);
>
> NEW
> // Return value represents caching behavior.
> // Naive strategy: return boolean. True = HttpRequest is a sufficient key
> to
> cache the rewrite. Works for simple cases, but compound rewriter cases may
> be more difficult.
> // More sophisticated implementation: return a RewriteCacheKey object,
> containing a normalized set of all inputs to the rewrite behavior
> (essentially a compound key).
> //    In turn, these objects would be used by a RewriterCache manager
> responsible for managing these states.
> ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
>
> Note that I suggest a new interface: ResponseRewriter (a more accurate name
> for RequestRewriter anyway), so we can support both Request and
> ResponseRewriter implementations during transition.
>
> Alternate approaches:
> I also considered simply splitting out the HTTP header-manipulation methods
> from HttpResponseBuilder into an interface: HttpResponseHeaders.
> RequestRewriter's API would be modified simply as:
> boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
> MutableContent);
>
> While simpler, this had the downsides:
> * Still involves a signature change.
> * Doesn't account for better caching behavior.
> * Involves 2 objects rather than 1 to manipulate for output.
>
>
> At present, I'd like to go the conservative route:
> 1. HttpResponseBuilder as MutableContent (@see CL)
> 2. Introduce ResponseRewriter interface, returning RewriteCacheKey with
> simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
> 3. Transition ImageRewriter to ResponseRewriter.
> 4. Transition RequestRewriters to ResponseRewriter interface.
>
>
> Thoughts? Input welcome.
>
> Cheers,
> John
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[Paul Lindner <li...@inuus.com>]

On Tue, May 4, 2010 at 1:00 PM, John Hjelmstad <fa...@google.com> wrote:

> We've implemented several RequestRewriters that aren't in Shindig, but
> they'd be trivial to refactor into ResponseRewriters as described here.
>
> I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
> BasicImageRewriter).
>
> What more-radical reorganization might you suggest? Any additional ideas
> beyond those suggested?
>

I don't have anything more radical, other than your proposed alternatives.
 My suggestions are to not worry too much about changing the interfaces if
it makes the code simpler/faster.


> At one point it seemed like a good idea to consolidate GadgetRewriter and
> Request||ResponseRewriter somehow, but the differences still seem notable
> enough as to disqualify that idea.
>
> --j
>
> On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com> wrote:
>
> > How many people have implemented custom rewriters?  I'm guessing it's a
> > small number, in which case I'd suggest a more radical reorganization of
> > the
> > code.
> >
> > In any case these are all good ideas.
> >
> > On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com>
> wrote:
> >
> > > Hi all,
> > >
> > > I'm looking into the rewriting pipeline, specifically for HTTP
> responses,
> > > with a few goals in mind:
> > > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
> > > one-off rewriter injection.
> > >  - And to simplify the rewriter interfaces overall, to allow for
> uniform
> > > rewriter profiling code etc.
> > > 2. Expand RequestRewriters' capability to modify HTTP response headers.
> > > 3. Maintain - and expand - our ability to cache the result of rewriting
> > > passes intelligently. Especially important for ImageRewriter, the
> output
> > of
> > > which is presently cached for notable performance gains.
> > >
> > > To this end, I've created the following CL as a starting point. It
> builds
> > > upon the CL I recently submitted adding the ability to manipulate
> > > MutableContent by byte-array:
> http://codereview.appspot.com/1032042/show
> > >
> > > Downstream of this CL, I'm proposing to change the rewriter interface
> > > roughly as follows:
> > >
> > > OLD
> > > // Return true if content modified.
> > > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
> > MutableContent);
> > >
> > > NEW
> > > // Return value represents caching behavior.
> > > // Naive strategy: return boolean. True = HttpRequest is a sufficient
> key
> > > to
> > > cache the rewrite. Works for simple cases, but compound rewriter cases
> > may
> > > be more difficult.
> > > // More sophisticated implementation: return a RewriteCacheKey object,
> > > containing a normalized set of all inputs to the rewrite behavior
> > > (essentially a compound key).
> > > //    In turn, these objects would be used by a RewriterCache manager
> > > responsible for managing these states.
> > > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
> > >
> > > Note that I suggest a new interface: ResponseRewriter (a more accurate
> > name
> > > for RequestRewriter anyway), so we can support both Request and
> > > ResponseRewriter implementations during transition.
> > >
> > > Alternate approaches:
> > > I also considered simply splitting out the HTTP header-manipulation
> > methods
> > > from HttpResponseBuilder into an interface: HttpResponseHeaders.
> > > RequestRewriter's API would be modified simply as:
> > > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
> > > MutableContent);
> > >
> > > While simpler, this had the downsides:
> > > * Still involves a signature change.
> > > * Doesn't account for better caching behavior.
> > > * Involves 2 objects rather than 1 to manipulate for output.
> > >
> > >
> > > At present, I'd like to go the conservative route:
> > > 1. HttpResponseBuilder as MutableContent (@see CL)
> > > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey with
> > > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
> > > 3. Transition ImageRewriter to ResponseRewriter.
> > > 4. Transition RequestRewriters to ResponseRewriter interface.
> > >
> > >
> > > Thoughts? Input welcome.
> > >
> > > Cheers,
> > > John
> > >
> >
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[John Hjelmstad <fa...@google.com>]

It does, very useful information. Anecdotally, we've seen the biggest gains
from cache extension as well, though naturally clients see a reasonable
improvement when being served a smaller image than they otherwise would be.

Cheers,
John

On Tue, May 11, 2010 at 2:04 PM, Chirag Shah <ch...@gmail.com> wrote:

> A) We are using a homegrown image rewriter because Shindig wasn't
> available when it was written.
> B) No caching is done by our rewriter. All of our image caching is
> done by Traffic Server.
> C) I would say our gains come from optimizations in Traffic Server.
>  - Using a TS plugin that implements stale-while-revalidate when the
> cached data is stale[1].
>  - Increasing the memory cache in TS.
>  - Rewriting Cache-Control headers to a minimum of 1 hour.
>
> Most of our notable gains came optimizations to our caching strategy.
> I hope this helps.
>
> [1] - http://www.mnot.net/blog/2007/12/12/stale
>
> Thanks,
> Chirag
>
>
>
> On Tue, May 4, 2010 at 7:18 PM, John Hjelmstad <fa...@google.com> wrote:
> > Cool --
> >
> > A) To what degree have you customized the ImageRewriter? Have you made it
> a
> > pipeline unto itself (ie. with sub-rewriters)? Doing so in a standard way
> is
> > a motivating factor for these proposals.
> > B) Are you modifying the caching behavior of the existing ImageRewriter
> > pipeline -- namely, that it's executed pre-caching?
> > C) To the extent you can say, where are your most notable gains coming
> from?
> > Optimization? Resizing? Caching? Downsampling (not impl'd in
> > BasicImageRewriter, but prototyped here)?
> >
> > FYI after some thinking about this, I'm putting together a CL that
> provides
> > a transition from RequestRewriter to ResponseRewriter, and transitions
> > ImageRewriter --> ResponseRewriter.
> >
> > ResponseRewriter's API, for the moment:
> > void rewrite(HttpRequest, HttpResponseBuilder);
> >
> > In my CL I'm @Inject'ing a ResponseRewriterRegistry with some annotation
> > (@Precache or similarly named) where the current ImageRewriter is
> injected,
> > for equivalent behavior.
> >
> > Note I'm starting w/ a return value of void to essentially punt on the
> > caching infrastructure, to allow this to be done later -- instead using
> the
> > status quo "where you inject is what caching you get" behavior.
> >
> > Obviously all this will be soon subject to codereview scrutiny :)
> >
> > --j
> >
> > On Tue, May 4, 2010 at 7:11 PM, Chirag Shah <ch...@gmail.com>
> wrote:
> >
> >> FWIW at Yahoo! we've configured a version of apache traffic server [1]
> >> to act as a reverse proxy which hits our own ImageRewriter. We've seen
> >> very notable performance gains with this setup.
> >>
> >> [1] http://trafficserver.apache.org/docs/v2/admin/reverse.htm
> >>
> >> On Tue, May 4, 2010 at 1:00 PM, John Hjelmstad <fa...@google.com>
> wrote:
> >> > We've implemented several RequestRewriters that aren't in Shindig, but
> >> > they'd be trivial to refactor into ResponseRewriters as described
> here.
> >> >
> >> > I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
> >> > BasicImageRewriter).
> >> >
> >> > What more-radical reorganization might you suggest? Any additional
> ideas
> >> > beyond those suggested?
> >> >
> >> > At one point it seemed like a good idea to consolidate GadgetRewriter
> and
> >> > Request||ResponseRewriter somehow, but the differences still seem
> notable
> >> > enough as to disqualify that idea.
> >> >
> >> > --j
> >> >
> >> > On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com>
> wrote:
> >> >
> >> >> How many people have implemented custom rewriters?  I'm guessing it's
> a
> >> >> small number, in which case I'd suggest a more radical reorganization
> of
> >> >> the
> >> >> code.
> >> >>
> >> >> In any case these are all good ideas.
> >> >>
> >> >> On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com>
> >> wrote:
> >> >>
> >> >> > Hi all,
> >> >> >
> >> >> > I'm looking into the rewriting pipeline, specifically for HTTP
> >> responses,
> >> >> > with a few goals in mind:
> >> >> > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just
> a
> >> >> > one-off rewriter injection.
> >> >> >  - And to simplify the rewriter interfaces overall, to allow for
> >> uniform
> >> >> > rewriter profiling code etc.
> >> >> > 2. Expand RequestRewriters' capability to modify HTTP response
> >> headers.
> >> >> > 3. Maintain - and expand - our ability to cache the result of
> >> rewriting
> >> >> > passes intelligently. Especially important for ImageRewriter, the
> >> output
> >> >> of
> >> >> > which is presently cached for notable performance gains.
> >> >> >
> >> >> > To this end, I've created the following CL as a starting point. It
> >> builds
> >> >> > upon the CL I recently submitted adding the ability to manipulate
> >> >> > MutableContent by byte-array:
> >> http://codereview.appspot.com/1032042/show
> >> >> >
> >> >> > Downstream of this CL, I'm proposing to change the rewriter
> interface
> >> >> > roughly as follows:
> >> >> >
> >> >> > OLD
> >> >> > // Return true if content modified.
> >> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
> >> >> MutableContent);
> >> >> >
> >> >> > NEW
> >> >> > // Return value represents caching behavior.
> >> >> > // Naive strategy: return boolean. True = HttpRequest is a
> sufficient
> >> key
> >> >> > to
> >> >> > cache the rewrite. Works for simple cases, but compound rewriter
> cases
> >> >> may
> >> >> > be more difficult.
> >> >> > // More sophisticated implementation: return a RewriteCacheKey
> object,
> >> >> > containing a normalized set of all inputs to the rewrite behavior
> >> >> > (essentially a compound key).
> >> >> > //    In turn, these objects would be used by a RewriterCache
> manager
> >> >> > responsible for managing these states.
> >> >> > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
> >> >> >
> >> >> > Note that I suggest a new interface: ResponseRewriter (a more
> accurate
> >> >> name
> >> >> > for RequestRewriter anyway), so we can support both Request and
> >> >> > ResponseRewriter implementations during transition.
> >> >> >
> >> >> > Alternate approaches:
> >> >> > I also considered simply splitting out the HTTP header-manipulation
> >> >> methods
> >> >> > from HttpResponseBuilder into an interface: HttpResponseHeaders.
> >> >> > RequestRewriter's API would be modified simply as:
> >> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
> >> >> > MutableContent);
> >> >> >
> >> >> > While simpler, this had the downsides:
> >> >> > * Still involves a signature change.
> >> >> > * Doesn't account for better caching behavior.
> >> >> > * Involves 2 objects rather than 1 to manipulate for output.
> >> >> >
> >> >> >
> >> >> > At present, I'd like to go the conservative route:
> >> >> > 1. HttpResponseBuilder as MutableContent (@see CL)
> >> >> > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey
> >> with
> >> >> > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
> >> >> > 3. Transition ImageRewriter to ResponseRewriter.
> >> >> > 4. Transition RequestRewriters to ResponseRewriter interface.
> >> >> >
> >> >> >
> >> >> > Thoughts? Input welcome.
> >> >> >
> >> >> > Cheers,
> >> >> > John
> >> >> >
> >> >>
> >> >
> >>
> >
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[Chirag Shah <ch...@gmail.com>]

A) We are using a homegrown image rewriter because Shindig wasn't
available when it was written.
B) No caching is done by our rewriter. All of our image caching is
done by Traffic Server.
C) I would say our gains come from optimizations in Traffic Server.
 - Using a TS plugin that implements stale-while-revalidate when the
cached data is stale[1].
 - Increasing the memory cache in TS.
 - Rewriting Cache-Control headers to a minimum of 1 hour.

Most of our notable gains came optimizations to our caching strategy.
I hope this helps.

[1] - http://www.mnot.net/blog/2007/12/12/stale

Thanks,
Chirag



On Tue, May 4, 2010 at 7:18 PM, John Hjelmstad <fa...@google.com> wrote:
> Cool --
>
> A) To what degree have you customized the ImageRewriter? Have you made it a
> pipeline unto itself (ie. with sub-rewriters)? Doing so in a standard way is
> a motivating factor for these proposals.
> B) Are you modifying the caching behavior of the existing ImageRewriter
> pipeline -- namely, that it's executed pre-caching?
> C) To the extent you can say, where are your most notable gains coming from?
> Optimization? Resizing? Caching? Downsampling (not impl'd in
> BasicImageRewriter, but prototyped here)?
>
> FYI after some thinking about this, I'm putting together a CL that provides
> a transition from RequestRewriter to ResponseRewriter, and transitions
> ImageRewriter --> ResponseRewriter.
>
> ResponseRewriter's API, for the moment:
> void rewrite(HttpRequest, HttpResponseBuilder);
>
> In my CL I'm @Inject'ing a ResponseRewriterRegistry with some annotation
> (@Precache or similarly named) where the current ImageRewriter is injected,
> for equivalent behavior.
>
> Note I'm starting w/ a return value of void to essentially punt on the
> caching infrastructure, to allow this to be done later -- instead using the
> status quo "where you inject is what caching you get" behavior.
>
> Obviously all this will be soon subject to codereview scrutiny :)
>
> --j
>
> On Tue, May 4, 2010 at 7:11 PM, Chirag Shah <ch...@gmail.com> wrote:
>
>> FWIW at Yahoo! we've configured a version of apache traffic server [1]
>> to act as a reverse proxy which hits our own ImageRewriter. We've seen
>> very notable performance gains with this setup.
>>
>> [1] http://trafficserver.apache.org/docs/v2/admin/reverse.htm
>>
>> On Tue, May 4, 2010 at 1:00 PM, John Hjelmstad <fa...@google.com> wrote:
>> > We've implemented several RequestRewriters that aren't in Shindig, but
>> > they'd be trivial to refactor into ResponseRewriters as described here.
>> >
>> > I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
>> > BasicImageRewriter).
>> >
>> > What more-radical reorganization might you suggest? Any additional ideas
>> > beyond those suggested?
>> >
>> > At one point it seemed like a good idea to consolidate GadgetRewriter and
>> > Request||ResponseRewriter somehow, but the differences still seem notable
>> > enough as to disqualify that idea.
>> >
>> > --j
>> >
>> > On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com> wrote:
>> >
>> >> How many people have implemented custom rewriters?  I'm guessing it's a
>> >> small number, in which case I'd suggest a more radical reorganization of
>> >> the
>> >> code.
>> >>
>> >> In any case these are all good ideas.
>> >>
>> >> On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com>
>> wrote:
>> >>
>> >> > Hi all,
>> >> >
>> >> > I'm looking into the rewriting pipeline, specifically for HTTP
>> responses,
>> >> > with a few goals in mind:
>> >> > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
>> >> > one-off rewriter injection.
>> >> >  - And to simplify the rewriter interfaces overall, to allow for
>> uniform
>> >> > rewriter profiling code etc.
>> >> > 2. Expand RequestRewriters' capability to modify HTTP response
>> headers.
>> >> > 3. Maintain - and expand - our ability to cache the result of
>> rewriting
>> >> > passes intelligently. Especially important for ImageRewriter, the
>> output
>> >> of
>> >> > which is presently cached for notable performance gains.
>> >> >
>> >> > To this end, I've created the following CL as a starting point. It
>> builds
>> >> > upon the CL I recently submitted adding the ability to manipulate
>> >> > MutableContent by byte-array:
>> http://codereview.appspot.com/1032042/show
>> >> >
>> >> > Downstream of this CL, I'm proposing to change the rewriter interface
>> >> > roughly as follows:
>> >> >
>> >> > OLD
>> >> > // Return true if content modified.
>> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
>> >> MutableContent);
>> >> >
>> >> > NEW
>> >> > // Return value represents caching behavior.
>> >> > // Naive strategy: return boolean. True = HttpRequest is a sufficient
>> key
>> >> > to
>> >> > cache the rewrite. Works for simple cases, but compound rewriter cases
>> >> may
>> >> > be more difficult.
>> >> > // More sophisticated implementation: return a RewriteCacheKey object,
>> >> > containing a normalized set of all inputs to the rewrite behavior
>> >> > (essentially a compound key).
>> >> > //    In turn, these objects would be used by a RewriterCache manager
>> >> > responsible for managing these states.
>> >> > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
>> >> >
>> >> > Note that I suggest a new interface: ResponseRewriter (a more accurate
>> >> name
>> >> > for RequestRewriter anyway), so we can support both Request and
>> >> > ResponseRewriter implementations during transition.
>> >> >
>> >> > Alternate approaches:
>> >> > I also considered simply splitting out the HTTP header-manipulation
>> >> methods
>> >> > from HttpResponseBuilder into an interface: HttpResponseHeaders.
>> >> > RequestRewriter's API would be modified simply as:
>> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
>> >> > MutableContent);
>> >> >
>> >> > While simpler, this had the downsides:
>> >> > * Still involves a signature change.
>> >> > * Doesn't account for better caching behavior.
>> >> > * Involves 2 objects rather than 1 to manipulate for output.
>> >> >
>> >> >
>> >> > At present, I'd like to go the conservative route:
>> >> > 1. HttpResponseBuilder as MutableContent (@see CL)
>> >> > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey
>> with
>> >> > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
>> >> > 3. Transition ImageRewriter to ResponseRewriter.
>> >> > 4. Transition RequestRewriters to ResponseRewriter interface.
>> >> >
>> >> >
>> >> > Thoughts? Input welcome.
>> >> >
>> >> > Cheers,
>> >> > John
>> >> >
>> >>
>> >
>>
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[John Hjelmstad <fa...@google.com>]

Cool --

A) To what degree have you customized the ImageRewriter? Have you made it a
pipeline unto itself (ie. with sub-rewriters)? Doing so in a standard way is
a motivating factor for these proposals.
B) Are you modifying the caching behavior of the existing ImageRewriter
pipeline -- namely, that it's executed pre-caching?
C) To the extent you can say, where are your most notable gains coming from?
Optimization? Resizing? Caching? Downsampling (not impl'd in
BasicImageRewriter, but prototyped here)?

FYI after some thinking about this, I'm putting together a CL that provides
a transition from RequestRewriter to ResponseRewriter, and transitions
ImageRewriter --> ResponseRewriter.

ResponseRewriter's API, for the moment:
void rewrite(HttpRequest, HttpResponseBuilder);

In my CL I'm @Inject'ing a ResponseRewriterRegistry with some annotation
(@Precache or similarly named) where the current ImageRewriter is injected,
for equivalent behavior.

Note I'm starting w/ a return value of void to essentially punt on the
caching infrastructure, to allow this to be done later -- instead using the
status quo "where you inject is what caching you get" behavior.

Obviously all this will be soon subject to codereview scrutiny :)

--j

On Tue, May 4, 2010 at 7:11 PM, Chirag Shah <ch...@gmail.com> wrote:

> FWIW at Yahoo! we've configured a version of apache traffic server [1]
> to act as a reverse proxy which hits our own ImageRewriter. We've seen
> very notable performance gains with this setup.
>
> [1] http://trafficserver.apache.org/docs/v2/admin/reverse.htm
>
> On Tue, May 4, 2010 at 1:00 PM, John Hjelmstad <fa...@google.com> wrote:
> > We've implemented several RequestRewriters that aren't in Shindig, but
> > they'd be trivial to refactor into ResponseRewriters as described here.
> >
> > I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
> > BasicImageRewriter).
> >
> > What more-radical reorganization might you suggest? Any additional ideas
> > beyond those suggested?
> >
> > At one point it seemed like a good idea to consolidate GadgetRewriter and
> > Request||ResponseRewriter somehow, but the differences still seem notable
> > enough as to disqualify that idea.
> >
> > --j
> >
> > On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com> wrote:
> >
> >> How many people have implemented custom rewriters?  I'm guessing it's a
> >> small number, in which case I'd suggest a more radical reorganization of
> >> the
> >> code.
> >>
> >> In any case these are all good ideas.
> >>
> >> On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com>
> wrote:
> >>
> >> > Hi all,
> >> >
> >> > I'm looking into the rewriting pipeline, specifically for HTTP
> responses,
> >> > with a few goals in mind:
> >> > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
> >> > one-off rewriter injection.
> >> >  - And to simplify the rewriter interfaces overall, to allow for
> uniform
> >> > rewriter profiling code etc.
> >> > 2. Expand RequestRewriters' capability to modify HTTP response
> headers.
> >> > 3. Maintain - and expand - our ability to cache the result of
> rewriting
> >> > passes intelligently. Especially important for ImageRewriter, the
> output
> >> of
> >> > which is presently cached for notable performance gains.
> >> >
> >> > To this end, I've created the following CL as a starting point. It
> builds
> >> > upon the CL I recently submitted adding the ability to manipulate
> >> > MutableContent by byte-array:
> http://codereview.appspot.com/1032042/show
> >> >
> >> > Downstream of this CL, I'm proposing to change the rewriter interface
> >> > roughly as follows:
> >> >
> >> > OLD
> >> > // Return true if content modified.
> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
> >> MutableContent);
> >> >
> >> > NEW
> >> > // Return value represents caching behavior.
> >> > // Naive strategy: return boolean. True = HttpRequest is a sufficient
> key
> >> > to
> >> > cache the rewrite. Works for simple cases, but compound rewriter cases
> >> may
> >> > be more difficult.
> >> > // More sophisticated implementation: return a RewriteCacheKey object,
> >> > containing a normalized set of all inputs to the rewrite behavior
> >> > (essentially a compound key).
> >> > //    In turn, these objects would be used by a RewriterCache manager
> >> > responsible for managing these states.
> >> > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
> >> >
> >> > Note that I suggest a new interface: ResponseRewriter (a more accurate
> >> name
> >> > for RequestRewriter anyway), so we can support both Request and
> >> > ResponseRewriter implementations during transition.
> >> >
> >> > Alternate approaches:
> >> > I also considered simply splitting out the HTTP header-manipulation
> >> methods
> >> > from HttpResponseBuilder into an interface: HttpResponseHeaders.
> >> > RequestRewriter's API would be modified simply as:
> >> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
> >> > MutableContent);
> >> >
> >> > While simpler, this had the downsides:
> >> > * Still involves a signature change.
> >> > * Doesn't account for better caching behavior.
> >> > * Involves 2 objects rather than 1 to manipulate for output.
> >> >
> >> >
> >> > At present, I'd like to go the conservative route:
> >> > 1. HttpResponseBuilder as MutableContent (@see CL)
> >> > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey
> with
> >> > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
> >> > 3. Transition ImageRewriter to ResponseRewriter.
> >> > 4. Transition RequestRewriters to ResponseRewriter interface.
> >> >
> >> >
> >> > Thoughts? Input welcome.
> >> >
> >> > Cheers,
> >> > John
> >> >
> >>
> >
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[Chirag Shah <ch...@gmail.com>]

FWIW at Yahoo! we've configured a version of apache traffic server [1]
to act as a reverse proxy which hits our own ImageRewriter. We've seen
very notable performance gains with this setup.

[1] http://trafficserver.apache.org/docs/v2/admin/reverse.htm

On Tue, May 4, 2010 at 1:00 PM, John Hjelmstad <fa...@google.com> wrote:
> We've implemented several RequestRewriters that aren't in Shindig, but
> they'd be trivial to refactor into ResponseRewriters as described here.
>
> I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
> BasicImageRewriter).
>
> What more-radical reorganization might you suggest? Any additional ideas
> beyond those suggested?
>
> At one point it seemed like a good idea to consolidate GadgetRewriter and
> Request||ResponseRewriter somehow, but the differences still seem notable
> enough as to disqualify that idea.
>
> --j
>
> On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com> wrote:
>
>> How many people have implemented custom rewriters?  I'm guessing it's a
>> small number, in which case I'd suggest a more radical reorganization of
>> the
>> code.
>>
>> In any case these are all good ideas.
>>
>> On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com> wrote:
>>
>> > Hi all,
>> >
>> > I'm looking into the rewriting pipeline, specifically for HTTP responses,
>> > with a few goals in mind:
>> > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
>> > one-off rewriter injection.
>> >  - And to simplify the rewriter interfaces overall, to allow for uniform
>> > rewriter profiling code etc.
>> > 2. Expand RequestRewriters' capability to modify HTTP response headers.
>> > 3. Maintain - and expand - our ability to cache the result of rewriting
>> > passes intelligently. Especially important for ImageRewriter, the output
>> of
>> > which is presently cached for notable performance gains.
>> >
>> > To this end, I've created the following CL as a starting point. It builds
>> > upon the CL I recently submitted adding the ability to manipulate
>> > MutableContent by byte-array: http://codereview.appspot.com/1032042/show
>> >
>> > Downstream of this CL, I'm proposing to change the rewriter interface
>> > roughly as follows:
>> >
>> > OLD
>> > // Return true if content modified.
>> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
>> MutableContent);
>> >
>> > NEW
>> > // Return value represents caching behavior.
>> > // Naive strategy: return boolean. True = HttpRequest is a sufficient key
>> > to
>> > cache the rewrite. Works for simple cases, but compound rewriter cases
>> may
>> > be more difficult.
>> > // More sophisticated implementation: return a RewriteCacheKey object,
>> > containing a normalized set of all inputs to the rewrite behavior
>> > (essentially a compound key).
>> > //    In turn, these objects would be used by a RewriterCache manager
>> > responsible for managing these states.
>> > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
>> >
>> > Note that I suggest a new interface: ResponseRewriter (a more accurate
>> name
>> > for RequestRewriter anyway), so we can support both Request and
>> > ResponseRewriter implementations during transition.
>> >
>> > Alternate approaches:
>> > I also considered simply splitting out the HTTP header-manipulation
>> methods
>> > from HttpResponseBuilder into an interface: HttpResponseHeaders.
>> > RequestRewriter's API would be modified simply as:
>> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
>> > MutableContent);
>> >
>> > While simpler, this had the downsides:
>> > * Still involves a signature change.
>> > * Doesn't account for better caching behavior.
>> > * Involves 2 objects rather than 1 to manipulate for output.
>> >
>> >
>> > At present, I'd like to go the conservative route:
>> > 1. HttpResponseBuilder as MutableContent (@see CL)
>> > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey with
>> > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
>> > 3. Transition ImageRewriter to ResponseRewriter.
>> > 4. Transition RequestRewriters to ResponseRewriter interface.
>> >
>> >
>> > Thoughts? Input welcome.
>> >
>> > Cheers,
>> > John
>> >
>>
>

Re: [Request || Response]Rewriters, ImageRewriters, and rewriter caching

[John Hjelmstad <fa...@google.com>]

We've implemented several RequestRewriters that aren't in Shindig, but
they'd be trivial to refactor into ResponseRewriters as described here.

I'm unaware of anyone who's implemented a custom ImageRewriter (beyond
BasicImageRewriter).

What more-radical reorganization might you suggest? Any additional ideas
beyond those suggested?

At one point it seemed like a good idea to consolidate GadgetRewriter and
Request||ResponseRewriter somehow, but the differences still seem notable
enough as to disqualify that idea.

--j

On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <li...@inuus.com> wrote:

> How many people have implemented custom rewriters?  I'm guessing it's a
> small number, in which case I'd suggest a more radical reorganization of
> the
> code.
>
> In any case these are all good ideas.
>
> On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com> wrote:
>
> > Hi all,
> >
> > I'm looking into the rewriting pipeline, specifically for HTTP responses,
> > with a few goals in mind:
> > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a
> > one-off rewriter injection.
> >  - And to simplify the rewriter interfaces overall, to allow for uniform
> > rewriter profiling code etc.
> > 2. Expand RequestRewriters' capability to modify HTTP response headers.
> > 3. Maintain - and expand - our ability to cache the result of rewriting
> > passes intelligently. Especially important for ImageRewriter, the output
> of
> > which is presently cached for notable performance gains.
> >
> > To this end, I've created the following CL as a starting point. It builds
> > upon the CL I recently submitted adding the ability to manipulate
> > MutableContent by byte-array: http://codereview.appspot.com/1032042/show
> >
> > Downstream of this CL, I'm proposing to change the rewriter interface
> > roughly as follows:
> >
> > OLD
> > // Return true if content modified.
> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse,
> MutableContent);
> >
> > NEW
> > // Return value represents caching behavior.
> > // Naive strategy: return boolean. True = HttpRequest is a sufficient key
> > to
> > cache the rewrite. Works for simple cases, but compound rewriter cases
> may
> > be more difficult.
> > // More sophisticated implementation: return a RewriteCacheKey object,
> > containing a normalized set of all inputs to the rewrite behavior
> > (essentially a compound key).
> > //    In turn, these objects would be used by a RewriterCache manager
> > responsible for managing these states.
> > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder);
> >
> > Note that I suggest a new interface: ResponseRewriter (a more accurate
> name
> > for RequestRewriter anyway), so we can support both Request and
> > ResponseRewriter implementations during transition.
> >
> > Alternate approaches:
> > I also considered simply splitting out the HTTP header-manipulation
> methods
> > from HttpResponseBuilder into an interface: HttpResponseHeaders.
> > RequestRewriter's API would be modified simply as:
> > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders,
> > MutableContent);
> >
> > While simpler, this had the downsides:
> > * Still involves a signature change.
> > * Doesn't account for better caching behavior.
> > * Involves 2 objects rather than 1 to manipulate for output.
> >
> >
> > At present, I'd like to go the conservative route:
> > 1. HttpResponseBuilder as MutableContent (@see CL)
> > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey with
> > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent.
> > 3. Transition ImageRewriter to ResponseRewriter.
> > 4. Transition RequestRewriters to ResponseRewriter interface.
> >
> >
> > Thoughts? Input welcome.
> >
> > Cheers,
> > John
> >
>