I had a lengthy conversation with catch today at BADCamp about how we're going to manage the process of getting the moving parts in place for the ESI/subrequest fanciness that we've been building toward. We are going to rejigger the way we're dealing with the various issues, so I'm documenting the roadmap here rather than in any of the specific issues.

The most important point is that catch said very firmly that he considers clean and working partial-page caching (via ESI and similar) to be a release-blocker for Drupal 8. We've done a ton of work so far to enable it, and we cannot release without it or we'll end up with performance regressions in Drupal 8. That, of course, is not acceptable. Therefore, the various moving parts for ESI are not 1 December feature-freeze sensitive, and we don't need to panic about getting them in before then. That allows us to change our strategy a bit. However, I would also appreciate confirmation from Dries and webchick that is the case. (Arguably it's making block caching actually work, and block caching is an existing feature, so it's not a new feature! Or something...)

Rather than put the existing generator code in now, we're going to postpone that on rebuilding path aliasing and output path caching. Therefore, the plan of attack is as follows:

  1. #1269742: Make path lookup code into a pluggable class - This is a critical bug, fixed by refactoring the path alias system into properly injectable libraries.
  2. Add a null implementation of the Generator, rather than the current implementation in #1705488: Implement a generator for Drupal paths. That null implementation is useful in that it allows us to wrap the matcher and the null generator into a Router object (a Symfony thing that is really just a combination of a matcher and a generator). Update: This is folded into the following task.
  3. With a Router, that gives us interface compatibility with the Symfony CMF Routing component. That means that, once the CMF Routing component merges in the PartialMatcher design we implemented, we can drop our code and use the CMF Routing component 98% verbatim. That gives us a shared routing component between Drupal, Symfony CMF, and ezPublish. Which is just awesome. Update: The CMF Router has been rewritten by Drupalers, and the code to update Drupal to use it is here: #1874500: CMF-based Routing system
  4. Once the path alias refactoring is in, we can then enhance the existing generator code to be more fully developed rather than a minimal implementation that is only practically useful for ESI. (See below for details of what that means.)
  5. We will hold off on #1808080: Add an _internal route for now as well. It may also get deprecated in favor of subclassing HttpKernel::getInternalPath() to hard-code the ESI internal path. TBD. Update: The HttpKernel class is being heavily refactored upstream in Symfony, so we're tracking that closely and will adjust accordingly once the dust settles.
  6. Once those are in place (either taking over getInternalPath() or a fully fleshed out generator, or both), we'll be able to issue HttpKernel::render() calls that will sort out themselves whether to use hInclude, ESI, or an in-process subreqest. And that will allow us to finally leverage HTTP itself to handle partial page caching in an intelligent way.
  7. #1597696: Consider whether HttpCache offers any significant benefit over the existing page cache Switch page caching over to HttpCache, so that we have a consistent cache API for everything.
  8. #1871596: Create a PartialResponse class to encapsulate html-level data [Moving to sandbox] Introduce a new way to define partial responses, which could be via subrequest or ESI or whatever else. (This is part of SCOTCH.)

The new, fully-developed generator (#1888424: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence) will incorporate the url() function as an additional method. Then, it will be modified to accept the event dispatcher as a dependency, allowing it to fire events. It will then fire an event for both route-based lookups and arbitrary URL lookups to allow event listeners to modify the path. Essentially, incorporating hook_url_outbound_alter() into all link generation calls. One of those listeners will be path aliasing, based on the cleaned up component above.

That normally would be a terrible idea for performance, but we are already caching alias lookups smartly. Instead, we'd extend the caching logic (another dependency for the generator) to wrap around all lookups, both route-based and arbitrary URL based. That gives us a single intelligent cache for all outgoing routes, so the typical case is cached with no event listener firing. (They may end up being two separate cache items; implementation detail.) We can then with more or less equal speed generate URLs based on routes or arbitrary links. Sweet.

Comments

sun’s picture

Title:[Meta] The ESI pipeline plan» [meta] The ESI pipeline battle plan
Category:task» feature
Priority:Major» Critical

1) I've tweaked the summary's formatting for readability.

2) I think we have sufficient critical/major bugs/tasks related to WSCCI/Kernel functionality in the queue already, and this is "just" a battle plan, so I hope you don't mind if I move this issue out of thresholds counters.

Crell’s picture

catch’s picture

Just to be clear on this bit:

The most important point is that catch said very firmly that he considers clean and working partial-page caching (via ESI and similar) to be a release-blocker for Drupal 8.

There's two things in terms of performance we need to do:

1. #1744302: [meta] Resolve known performance regressions in Drupal 8 - i.e. optimize things that are new and known to be slow.

2. I don't think #1 is going to be enough to get us to parity with Drupal 7, and the biggest single win we can make for performance is caching rendered HTML. This means using render/block caching for more things, and more things means menu blocks and/or entity renders (which might be the same thing once menus and menu items are entities). So the really critical thing is 'turn on caching for more fully rendered stuff", the fact it works with reverse proxies/ESI/SSI should be a side-effect of this by the time we're done.

effulgentsia’s picture

Given that #3.2 mentions converting non-block thingies into blocks as part of the scope of this meta issue, I downgraded #1696302: [meta] Blocks/Layouts everywhere from critical to major, since the rest of what that issue covers either already has specific issues classified as critical, or is not actually critical.

Dries’s picture

I'd still really like to see ESI in Drupal 8, but am a bit worried with the momentum behind this. I don't see anyone taking the lead on this. I just committed #1535868: Convert all blocks into plugins - maybe that will spark some more people to work on ESI.

Crell’s picture

The currently-active issue related to this is #1874500: CMF-based Routing system, which is item 3 above. We kept trying to get incremental pieces in place but catch kept rejecting "this doesn't do anything yet but builds for later" patches, so now it will just be one huge patch. The other big piece will be Sam's layout/display patch, which is also in progress but off the grid.

Crell’s picture

Issue summary:View changes

Formatting.

Wim Leers’s picture

#6: So that means that only item 1 is done and item 3 (which according to the summary includes item 2) is being worked on? How far are we, if you'd express progress in a percentage?

Crell’s picture

I'd say > 50% of the patch for step 3? Go see the issue for way more details than I can repeat here, and please review it. :-)

Wim Leers’s picture

Sorry, I meant a percentage for the overall progress for ESI support, not for that specific issue. It's very, very hard to get a feel for that if you're not closely involved!

catch’s picture

We kept trying to get incremental pieces in place but catch kept rejecting "this doesn't do anything yet but builds for later" patches,

Eh? I've committed lots of those. If you mean the generator patch, that was completely broken, not "doesn't do anything yet").

webchick’s picture

Here mostly to copy/paste Dries's comment in #5 and Wim Leers's comment in #9. :)

It'd be great to see the issue summary updated for what's actually left as part of this initiative. It seems like many/most(?) of the issues in the issue summary are done, but yet there still hasn't been any real traction on this over-arching mission that I've seen? It seems like Kris and Sam are pretty swamped and could really use a hand.

catch’s picture

Crell’s picture

There's now a new patch and movement in #1597696-63: Consider whether HttpCache offers any significant benefit over the existing page cache for those who want something to review.

Crell’s picture

Issue summary:View changes

Update for recent developments.

Crell’s picture

Issue summary:View changes

Update for more recent developments.

Crell’s picture

Assigned:Unassigned» catch

catch: Should we close this issue as redundant now with work happening elsewhere?

Crell’s picture

Issue summary:View changes

Switch from strike to del tags.

catch’s picture

Issue summary:View changes
Status:Active» Closed (duplicate)

I think this can be closed now yeah - we've got issues open for pretty much every bit of it, and if they aren't then they're probably missing from here too.