Problem/Motivation

Comment::hasParentComment() checks if there is a parent comment, and it does so by checking if it can load an entity.

That has two problems:
a) we might be loading the entity when we don't need it.
b) The default value for that field is 0, which then does a load(0), this is currently not being cached.

To reproduce:

Enable threaded comments, create a node with a bunch of comments without a parent.

Proposed resolution

If possible only check for ->target_id, or at least check that before checking ->entity.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#1 comment-has-parent-2328629-1.patch524 bytesBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
comment-has-parent-2328629-1.patch524 bytes

Here's a patch.

moshe weitzman’s picture

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

Nice performance fix. Thanks.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed c16c1c7 on 8.0.x 
    Issue #2328629 by Berdir: Fixed Comment::hasParentComment() tries to...

Status: Fixed » Closed (fixed)

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