Agreed, there's a good deal of trimming rpc core could use. I'll take it up in the patch comments.
On Thu, Jun 11, 2009 at 1:34 AM, Kevin Brown <[email protected]> wrote: > On Wed, Jun 10, 2009 at 6:00 PM, John Hjelmstad <[email protected]> > wrote: > > > I recognize there are other fruitful optimizations we can do, and plan on > > tackling several more over time. > > > > With this idea, I'm aiming at reducing the container JS footprint. That > the > > gadget footprint would also be reduced is a bonus, but clearly we have a > > big > > mess of JS there (~45kB of core alone, by a quick count) of which rpc's > > fraction is relatively small. Opensocial-0.[8,9] is another ball of wax, > > but > > remember Shindig renders non-OS gadgets as well. > > > > So why look at optimizing rpc: it's commonly included by containers, its > > use > > is well-circumscribed (all the containers use is gadgets.rpc.* -- core > > gadget JS optimization, to take one example, requires assurances that no > > gadgets.util/config/json/log/prefs/io methods are ever executed by the > > gadget without any declaration on its part), and the container page is > > sacred ground for latency. > > > > Taking a look at the numbers... the optimizations apply to refactored > > rpc.js > > including RMR. > > > > That clocks in at: > > 557 wpm.transport.opt.js > > 4466 rpc.opt.js > > 2825 rmr.transport.opt.js > > 1998 nix.transport.opt.js > > 1292 ifpc.transport.opt.js > > 1005 fe.transport.opt.js > > 534 dpm.transport.opt.js > > > > Total: 12,677B, gzipped: 4,365 > > > > Other iterations- > > All minus IFPC: 11,385 raw, 3,948 gzip > > > > > WPM only: 5,023 raw, 2,051 gzip > > DPM only: 5,000 raw, 2,043 gzip > > > DPM should simply be removed. I'm sure all 5 people still using Opera 8 > will > be upset. > > You've also still got stuff like callSameDomain bloating up the core, when > really it should only be included for channels where it actually has a > performance advantage. Ditto get / set relay url, get / set authToken, > receive, and the 'legacy' stuff. By my estimate, you eliminate half of > rpc.js when you remove ifpc.js. I'll add these comments to the actual > review > of the patch though :). > > > > > > NIX only: 6,464 raw, 2,550 gzip > > FE only: 5,471 raw, 2,158 gzip > > RMR only: 7,291 raw, 2,951 gzip > > > > As Balaji notes, 15% of users don't have gzip. So the savings are > somewhere > > in the ballpark of ~2kB gzip and 7kB raw. The heuristic conclusion of > > several analyses I've read is that 3000 bytes = ~60ms added page latency. > I > > want to reduce that as much as possible. > > > > Re: browser passing along the filter. I've experimented a bit with this, > > and > > it can indeed work but with a few pain points. Essentially, the technique > > moves the getRelayChannel() method into a snippet of JS which constructs > a > > script URL eg: > > <script> > > document.write("<script src=' > > > http://www.gadgetserver.com/gadgets/js/rpc.js?c=1&container=mycontainer&tx= > > " > > + getRelayChannel() + "'></script>"); > > </script> > > > > > > Issues: > > 1. Brittle: the implementation of getRelayChannel() needs to be updated > for > > every container that's hardcoded it if any changes are needed. > > - Would prefer to keep integration points tightly controlled. > > 2. Hard-coded URL generation. > > - Mitigated by server generation of URL, which happens on many > containers. > > For those, we can implement this technique later, building it on this > code. > > 3. Forced additional script load. > > - Server-side support allows server-to-server retrieval of per-browser > JS > > with dynamic compilation inline into container JS to avoid additional > HTTP > > request. > > > > Because the work in this CL can trivially be utilized by this technique, > I > > thought it best to implement this first and wait until the rpc library > > stabilizes for some time before introducing another integration method. > > > > I share your concern about IE UA faking. That's one reason this code is > an > > alternative but not a requirement. Two possible solutions: 1. In-browser > > technique selection (as noted above), 2. For IE UAs that are faked (all? > > Just IE6/7?), emit ALL_TX rather than NIX_TX. > > > > > > > --John > > > > On Wed, Jun 10, 2009 at 10:45 AM, Kevin Brown <[email protected]> wrote: > > > > > Getting rid of IFPC brings the gzipped size down to around 2k, which > was > > my > > > personal preference. > > > > > > Compared to the opensocial-0.8, or even 'core', though, rpc is pretty > > > small. Those libraries weigh in at 30k and 10k, respectively. > > > > > > > > > On Wed, Jun 10, 2009 at 7:06 AM, Paul Lindner <[email protected]> > wrote: > > > > > >> Of all the optimizations out there this seems like the least > interesting > > >> and > > >> more disruptive than necessary. The (current) whole of rpc.js is > > >> 6285 Jun 8 13:28 ./features/target/classes/features/rpc/rpc.opt.js > > >> > > >> Making this 6k private instead of CDN cacheable would mean that I > would > > >> have > > >> to find another 6k to move onto the CDN to insure that I (and others) > > hit > > >> our cold-cache numbers for loading profile / home pages. > > >> > > >> Do you mind sharing your goals here? What kind of byte-savings are > you > > >> expecting? > > >> > > >> Is it not possible to have the browser send a filtering parameter > along > > >> with > > >> the request? The getRelayChannel() function figures out your > > capabilities > > >> based on what the browser can do, not the value of it's > > >> UA. For example there are some people that change their User-Agent to > > >> (mostly) Internet Explorer to get around server restrictions. These > > >> people > > >> would get broken nix. > > >> > > >> I just wonder if there are other ways to achieve the wanted byte > > savings? > > >> > > >> > > >> On Tue, Jun 9, 2009 at 2:18 PM, John Hjelmstad <[email protected]> > > wrote: > > >> > > >> > I'm sold, relying on Cache-Control: private is the better option. > > >> > This behavior does affect IFRAME rendering paths as well, though > > doesn't > > >> > necessarily need to. Container-side rpc can be optimized without > > >> > gadget-side > > >> > rpc, since gadgets.rpc(function getRelayChannel()) will return the > > same > > >> > value on each side. > > >> > > > >> > As Brian mentions, every code path in GadgetRenderingServlet sets > > >> > Cache-Control: private[,*] (except nocache=1, not an issue), so the > > >> > potential effects are limited to JsServlet. > > >> > > > >> > So 1. Admittedly, any given deployment will likely see higher > > JsServlet > > >> > traffic if choosing to opt into this feature; 2. this suggests that > > >> > JsLibrary field isBrowserSpecific should be added, with JsServlet > > >> signaling > > >> > noProxy behavior in caching headers when this bit is true for any > > >> library > > >> > it > > >> > emits. > > >> > > > >> > John > > >> > > > >> > On Tue, Jun 9, 2009 at 1:53 PM, Brian Eaton <[email protected]> > > wrote: > > >> > > > >> > > I don't trust the vary header any farther than I can spit. > > >> > > > > >> > > For example, at least one version of IE interprets a Vary header > > with > > >> > > any value other than user-agent to mean "OMG, I don't understand, > > >> > > don't cache this": > > >> > > http://marc.info/?l=apache-modgzip&m=103958533520502&w=2 > > >> > > > > >> > > In a comment to Mark's blog post, he mentions other implementation > > >> > issues: > > >> > > > > >> > > "More complex situations -- e.g., with multiple Vary headers and > > >> > > multiple request headers -- caused problems on a number of > > >> > > implementations (i.e., they'd return the wrong thing)." > > >> > > > > >> > > IMHO, we should stick to "cache-control: private." > > >> > > > > >> > > We need that on most iframes anyway, so this keeps the > cache-control > > >> > > changes from rippling out quite so far. > > >> > > > > >> > > On Tue, Jun 9, 2009 at 1:44 PM, John Panzer<[email protected]> > > >> wrote: > > >> > > > This seems relevant: > > >> http://www.mnot.net/blog/2007/06/20/proxy_caching > > >> > > > (It appears that Vary: is fairly safe in the sense that caches > > that > > >> > don't > > >> > > > deal with it nicely -- e.g., caching variants -- will just mark > > the > > >> > > content > > >> > > > as uncacheable. Though I don't think this was rigorously > tested.) > > >> > > > > > >> > > > -- > > >> > > > John Panzer / Blogger > > >> > > > [email protected] / abstractioneer.org < > > >> > http://www.abstractioneer.org/> > > >> > > / > > >> > > > @jpanzer > > >> > > > > > >> > > > > > >> > > > > > >> > > > On Tue, Jun 9, 2009 at 1:25 PM, <[email protected]> wrote: > > >> > > > > > >> > > >> Hi Paul: > > >> > > >> > > >> > > >> You're absolutely right. Vary: User-Agent; is preferable to me. > > Do > > >> you > > >> > > >> have any reason in mind for why Cache-Control: private in > > addition > > >> to > > >> > or > > >> > > >> instead of Vary would be preferable? Ie. common support by > > various > > >> > CDNs. > > >> > > >> > > >> > > >> I've been thinking how best to link service of custom rpc to > > >> setting > > >> > > >> this header. Ultimately JsServlet or a Filter needs to do so. > > >> Thoughts > > >> > > >> on this? The best idea I had was to add something like > > >> > > >> isBrowserSpecific() to JsLibrary so that the relevant output > code > > >> > > >> (gadget rendering, js servlet) could act accordingly. > > >> > > >> > > >> > > >> Kevin - re: StringBuffer, comment isn't particularly > prescriptive > > >> so > > >> > > >> I'll assume the complaint is use of StringBuffer rather than > > >> > > >> StringBuilder. If so, fixed. If not, let me know. > > >> > > >> > > >> > > >> > > >> > > >> http://codereview.appspot.com/63210 > > >> > > >> > > >> > > > > > >> > > > > >> > > > >> > > > > > > > > >
