Protected property $dateFormat in Drupal\datetime\Plugin\views\filter\Date is assigned the value of DATETIME_DATETIME_STORAGE_FORMAT when declared, then the same value is assigned again if certain conditions are met in the constructor and then it's used in read only mode only. This clearly can be optimized. I also did it with patch #7 from https://www.drupal.org/node/2912647 applied which is replaces the datetime.module constants with the same in DateTimeInterface constants.

CommentFileSizeAuthor
#2 interdiff-2912647-2912749.txt3.63 KBangel.h
#2 2912749-2-code_optimization.patch52.06 KBangel.h
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

angel.h created an issue. See original summary.

angel.h’s picture

angel.h
Primary language English
Location Plovdiv
CreditAttribution: angel.h at FFW commented
Assigned: angel.h » Unassigned
Status: Needs work » Needs review
FileSize
2912749-2-code_optimization.patch52.06 KB
interdiff-2912647-2912749.txt3.63 KB

Here is the patch with #7 from https://www.drupal.org/node/2912647 applied before that and the interdiff between the patch for this issue and #7 from 2912647.

angel.h’s picture

angel.h
Primary language English
Location Plovdiv
CreditAttribution: angel.h at FFW commented
Title: Code optimization in Drupal\datetime\Plugin\views\filter\Date » Code optimization in \Drupal\datetime\Plugin\views\filter\Date
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: 2912749-2-code_optimization.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Related issues: +#2826404: Create DateTimeItemInterface and deprecate global constants in datetime.module

Thanks for the patch, but I am not really sure if this optimization is worth it.

  1. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
@@ -32,15 +33,6 @@ class Date extends NumericDate implements ContainerFactoryPluginInterface {
-  /**

@@ -65,12 +57,6 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
-    $definition = $this->getFieldStorageDefinition();
...
   }

    This is a BC break for anything that may have extended this.

  2. +++ b/core/modules/datetime/src/Plugin/views/filter/Date.php
@@ -65,12 +57,6 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
-
-    // Date format depends on field storage format.
-    $definition = $this->getFieldStorageDefinition();
-    if ($definition->getSetting('datetime_type') === DateTimeItem::DATETIME_TYPE_DATE) {
-      $this->dateFormat = DATETIME_DATE_STORAGE_FORMAT;
-    }

    This isn't a valid change. $this->dateFormat needs to vary, depending on what the field config is.

With all of the changes to the interface, this is really hard to evaluate. All of the changes to move the constants to the interface are out of scope for this issue.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Status: Needs work » Closed (works as designed)

Cleaning up some issues. I looked at this patch again, and the logic that it tries to optimized out is needed to handle datetime/date+only variants. So, closing this for the time being.