QueryString::buildUrls should not rebuild requests for the same facet source path

The fix looks something like this:

    if ($facet_source_path) {
      if (isset($this->requestsByPath[$facet_source_path])) {
        $request = $this->requestsByPath[$facet_source_path];
      }
      else {
        $request = Request::create($facet_source_path);
        $request->attributes->set('_format', $this->request->get('_format'));
        $this->requestsByPath[$facet_source_path] = $request;
      }
    }

Git is currently down. I'll roll a patch later.

Here is a patch. I'll deploy it and run Blackfire.

Great! I've been getting a regular release for the project out. I'm still aiming to post an "after" profile.

Sorry, I don't know what i'm thinking. ignore this comment.

Okay, I'm not crazy. The original patch did not work. The class is always instantiated as a new object, blowing the cache. We need a static variable in the method itself to preserve the cache.

Okay, there is more to it. It's also the large amount of calls to Url::createFromRequest.

This patch uses more caching, but it isn't elegant. I need to use this for now, but I'd like to find a better approach.

Maybe injecting cache.static? Or just a more elegant way of handling the checks.

The problem is this:

• Constantly recreating request objects
• Rebuilding the same URL via createFromRequest for each facet URL. Each facet URL needs two of these generated.

Okay, I'll clean it up so it makes more sense. We've had this in production for a week or so now. With 600 Facet objects the page went from 13s to 9s in Blackfire.

Okay, here is a cleaned up patch. Instead of stuffing static entries into the one method, I broke things out a bit. I felt like this made it easier to document and a less confusing. Otherwise, we would be defining static variables at the beginning of the method and using them sometime after, making it hard to comprehend -- especially since this pattern is not as common in Drupal as it is in PHP as a whole.