At the moment there is quite some logic going on, to decide whether to capture the views time etc.

This probably should be all moved into viewUI, so it's not executed on the actual page:

   $config = config('views.settings');

    // Set the response so other parts can alter it.
    $this->response = new Response('', 200);

    $start = microtime(TRUE);
    if (!empty($this->live_preview) && $config->get('ui.show.additional_queries')) {
      $this->startQueryCapture();
    }

    $exposed_form = $this->display_handler->getPlugin('exposed_form');
    $exposed_form->pre_render($this->result);

    // Check for already-cached output.
    if (!empty($this->live_preview)) {
      $cache = FALSE;
    }
CommentFileSizeAuthor
#20 drupal-1811982-20.patch7.17 KBdawehner
#18 drupal-1811982-18.patch5.58 KBdawehner
#17 drupal-1849356-37.patch6.97 KBdawehner
#13 interdiff.txt903 bytesdawehner
#13 drupal-1811982-13.patch5.6 KBdawehner
#7 drupal-1811982-7.patch5.59 KBdamiankloip
#7 interdiff.txt896 bytesdamiankloip
#4 drupal-1811982-4.patch5.67 KBdawehner
#1 1811982.patch1.95 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Active » Needs review
FileSize
1811982.patch1.95 KB

This patch moves this logic from render on ViewExecutable to renderPreview on the ViewUI object. This is a better place for it I think. We can also lose the check for live_preview.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Mh people actually seems to use this #1809510: Make render time performance metric accessible in hook_views_post_render()

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1811982-4.patch5.67 KB

Let's rerole and clean it up a bit.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Let's rerole and clean it up a bit.

Status: Needs review » Needs work

The last submitted patch, drupal-1811982-4.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
interdiff.txt896 bytes
drupal-1811982-7.patch5.59 KB

I think a couple of the calls were on the ViewUI and not executable and vice versa.

Status: Needs review » Needs work
Issue tags: -VDC

The last submitted patch, drupal-1811982-7.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
Issue tags: +VDC

#7: drupal-1811982-7.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Cool!

Dries’s picture

Dries CreditAttribution: Dries commented

Doesn't Drupal core have some timer functionality that may be re-usable here? I kinda forgot, to be honest.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Reviewed & tested by the community » Needs work

dries++ Wow that's pretty cool.

Working on that.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1811982-13.patch5.6 KB
interdiff.txt903 bytes

There we go.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs review » Reviewed & tested by the community

Looks good again to me, it's using the timer stuff that it seems we nearly all forgot about :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Issue tags: -VDC

#13: drupal-1811982-13.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +VDC

The last submitted patch, drupal-1811982-13.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Reviewed & tested by the community
FileSize
drupal-1849356-37.patch6.97 KB

Just another rerole. back to rtbc

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1811982-18.patch5.58 KB

Nothing complicated

patch -p1 < drupal-1811982-13.patch 
patching file core/modules/views/lib/Drupal/views/ViewExecutable.php
Hunk #1 succeeded at 1227 with fuzz 2 (offset -3 lines).
Hunk #2 succeeded at 1324 (offset 2 lines).
Hunk #3 succeeded at 1692 (offset 2 lines).
patching file core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented 
+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
@@ -579,6 +579,56 @@ public function submitItemAdd($form, &$form_state) {
+    // make a copy of the array so we can manipulate it with array_splice.

Not sure this comment is accurate now?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Reviewed & tested by the community » Needs review
FileSize
drupal-1811982-20.patch7.17 KB

Let's rewrite it, so it does actually work.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

#20: drupal-1811982-20.patch queued for re-testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs review » Reviewed & tested by the community

Just tested this manually, works great.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 7.x-3.x-dev
Component: views.module » Code
Status: Reviewed & tested by the community » Patch (to be ported)

Ooh much nicer. Committed/pushed to 8.x.