Http cache

[vicb]

Http has the following code:

      // We return a pending request only if caching is enabled.
      if (cache != null && _pendingRequests.containsKey(url)) {
        return _pendingRequests[url];
      }

      var cachedResponse = (cache != null && method == 'GET') ? cache.get(url) : null;
      if (cachedResponse != null) {
        return new async.Future.value(new HttpResponse.copy(cachedResponse));
      }

I think there are several issues here:

• We should return the pending requests only for idempotent methods - let's way we issue to consecutive POST requests to the same URL to create 2 resources, only 1 will be created,
• We should also consider HEAD (ie all safe methods) when returning a cache response,
• We should also take cache headers into account when returning a cached response. With the naive cache we have today, we might return an outdated response for GET request.

References:

• HTTP spec section 9.1 for safe & idempotent methods,
• HTTP spec section 13 for caching

[vsavkin]

@vicb If we were to build a full featured cache, do you think it should be a part of Angular? From what I can see, there is nothing angular-specific about how the cache is used right now. So maybe writing a decorator for dom.HttpRequest.request would be better. In this case, non Angular projects would be able to use it too.

[vicb]

It should definitely not be part of angular.

My idea is that the code we are looking for is probably already living somewhere in dart so apparently this is request.

Let me have a look at b/o next week and let's discuss the best way to do it.

[vicb]

Actually we should probably only rely on the browser cache. The current implementation is very naive and violates the http cache spec in many ways.
What about removing the cache layer here ?

[vicb]

An other been benefit is that debugging would be much easier as all requests would appear in the dev tools

[vsavkin]

IMO using the browser cache should be OK. So I think removing the cache layer, at least for now, is a good approach. BTW, AngularJS has the same naive implementation of cache.

[jbdeboer]

How much slower is getting something from the browser cache? You need to
switch out of the VM, which will result in an extra Scope.apply().

I would love to see numbers, though.

AngularJS has this same problem, the cache policy is horrible and generally
broken.

+1 for a separate, compliant caching module. This is fairly low priority
at the moment: most apps that I know of implement their own data RPC system
and don't use this cache for anything with complicated caching requirements.

On Sat, May 31, 2014 at 7:31 AM, Victor Savkin [email protected]
wrote:

IMO using the browser cache should be fine.

—
Reply to this email directly or view it on GitHub
#1076 (comment)
.

[vicb]

I think there are 2 separate concerns here: functionality and performance.

There is no way to fix the bugs of the current implementation with the current interface. A proper cache should have a request as input and not an URL (to access the headers). Then I propose that we start by dropping the current cache asap.

I have to think more about the perf. I don't really see how we can avoid an apply after each single request (don't this assume that only requests can update the data ?). For sure dropping the cache would save memory.

On June 1, 2014 5:39:00 PM CEST, James deBoer [email protected] wrote:

How much slower is getting something from the browser cache? You need
to
switch out of the VM, which will result in an extra Scope.apply().

I would love to see numbers, though.

AngularJS has this same problem, the cache policy is horrible and
generally
broken.

+1 for a separate, compliant caching module. This is fairly low
priority
at the moment: most apps that I know of implement their own data RPC
system
and don't use this cache for anything with complicated caching
requirements.

On Sat, May 31, 2014 at 7:31 AM, Victor Savkin
[email protected]
wrote:

IMO using the browser cache should be fine.

—
Reply to this email directly or view it on GitHub

#1076 (comment)
.

---

Reply to this email directly or view it on GitHub:
#1076 (comment)

[vsavkin]

My experiments showed that using the browser cache is about 8 times slower (which is about 3ms per request). I think 3ms is fine, cause we are not talking about any real-time data here (it would not be cached anyways).

I agree with @vicb that the current cache should be dropped. It is basically broken and can cause some hard to troubleshoot bugs.

[vicb]

👍 to drop the current cache. IMO 3ms should not be compared with a in-memory cache but to the HTTP round trip time (worst case).

We could also add provision for a TBD cache system (as the API is known).

[vsavkin]

I can submit a PR dropping the cache.

[vicb]

That would be great

[jbdeboer]

IIUC, templates are cached using the Http cache, so 3ms * 1000s of template lookups is a deal-breaker.

#1107 should address that somewhat and reduce it to 3ms * 100s of component classes. 3ms * 100 is still too high.

[vsavkin]

That's a very good point.

Right now, TemplateCache is used like this:

http.get(cssUrl, cache: templateCache)

Can we change it, so it is used like this?

templateCache.get(cssUrl)

Where templateCache can use the Http service to get the data, and then cache it (as a String, not an HttpResponse).

IMO it leads to a few nice things:

First, since we do not cache http responses, but templates, we do not have to follow the http spec. So we can use the naive caching we have today (templates do not get changed anyways).

Second, it makes preloading templates less confusing. Cause right now if I have something like this in an html file:

<template id="my_template.html" type="text/ng-template">
  My Template
</template>

It will be cached as an http response, though no requests have been made.