Rendering the comment form for authenticated users happens via #post_render_cache, because the form is personalized for the current user. But, to build the comment form, we need a comment entity. And to build a comment entity, we must use ::create(), which will call ::applyDefaultValues(), which is expensive because of all the Typed Data things (IIRC) it needs in order to function.

  public function renderForm(array $element, array $context) {
    $field_name = $context['field_name'];
    $entity = $this->entityManager->getStorage($context['entity_type'])->load($context['entity_id']);
    $field_storage = FieldStorageConfig::loadByName($entity->getEntityTypeId(), $field_name);
    $values = array(
      'entity_type' => $entity->getEntityTypeId(),
      'entity_id' => $entity->id(),
      'field_name' => $field_name,
      'comment_type' => $field_storage->getSetting('bundle'),
      'pid' => NULL,
    );
    $comment = $this->entityManager->getStorage('comment')->create($values);
    $form = $this->entityFormBuilder->getForm($comment);
    $markup = $this->renderer->render($form);

    $callback = 'comment.post_render_cache:renderForm';
    $placeholder = $this->generatePlaceholder($callback, $context);
    $element['#markup'] = str_replace($placeholder, $markup, $element['#markup']);
    $element = $this->renderer->mergeBubbleableMetadata($element, $form);

    return $element;
  }

Applying the same trick as in #2458817: Creating new user entities for anonymous users is very slow is impossible, because it's very likely for comment entities to have additional fields, and because we are showing a form, we need those default values to be applied (so that their widgets have the expected default values)!

CommentFileSizeAuthor
#2 2471218.patch2.44 KBamateescu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

@fago, @Berdir, did we have an idea on how to solve this?

amateescu’s picture

Status: Active » Needs review
FileSize
2.44 KB

Discussed this a bit a few days ago with @Berdir and we simply need a full content entity in order to generate a form for it.

So all we can do here is optimize CommentPostRenderCache::renderForm() to not do any processing that's already done by CommentDefaultFormatter::viewElements.

1 cut down, 999 to go :)

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Quickfix
  1. +++ b/core/modules/comment/src/CommentPostRenderCache.php
    @@ -99,19 +98,17 @@ public function __construct(EntityManagerInterface $entity_manager, EntityFormBu
    -    $field_name = $context['field_name'];
    -    $entity = $this->entityManager->getStorage($context['entity_type'])->load($context['entity_id']);
    -    $field_storage = FieldStorageConfig::loadByName($entity->getEntityTypeId(), $field_name);
         $values = array(
    -      'entity_type' => $entity->getEntityTypeId(),
    -      'entity_id' => $entity->id(),
    -      'field_name' => $field_name,
    -      'comment_type' => $field_storage->getSetting('bundle'),
    +      'entity_type' => $context['entity_type'],
    +      'entity_id' => $context['entity_id'],
    +      'field_name' => $context['field_name'],
    +      'comment_type' => $context['comment_type'],
    

    Yay!

  2. +++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/CommentDefaultFormatter.php
    @@ -204,6 +204,7 @@ public function viewElements(FieldItemListInterface $items) {
    +              'comment_type' => $this->getFieldSetting('comment_type'),
    

    Look at that, I guess we forgot that… or was it newly added at a later time perhaps?

    In any case: thanks!


This looks beautifully simple!

xjm’s picture

Status: Reviewed & tested by the community » Fixed

This makes sense. Committed and pushed to 8.0.x. Thanks!

  • xjm committed 13e4f31 on 8.0.x
    Issue #2471218 by amateescu, Wim Leers: Dummy comment entity necessary...
Wim Leers’s picture

This saved 2 milliseconds on /node/1 :)

Fabianx’s picture

So it does not make sense to create cached default values for such entities? Provided the TypedData things can be cached somehow?

andypost’s picture

@Fabianx seems yes, comment render/caching needs separate issue
There's already
#2002094: Improve performance of comment.html.twig
#1857946: Comment parent template variables are built twice

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.