Improve views field rendering performance by caching entity display objects

So, yes, the perf of "field-by-field" Views, currently based on individual $item->view() calls, is crazy poor. We've had a long standing plan to make it better, it just never got to the top of the todo lists so far :-). The issue is #1867518: Leverage entityDisplay to provide fast rendering for fields.

Basically, the idea is a View should :
- run its query,
- load the corresponding entities,
- create a runtime EntityViewDisplay object, and configure it with ->setComponent() on each of the N fields it needs to display. This happens only once.
- render the entities in one pass: $output = EVB::buildMultiple($entities)
- for each row/entity and each field in the row, pick the output of the field in the $output array: $field_output = $output[$entity_id][$field_name]
(in practice, this is a bit more complicated because of views relationships and stuff like that, but that's the idea)

I still think this is the right approach (instanciates the formatters only once for all entities, benefits on multiple rendering), but this does not exclude the optimizations done in the patch here, that will benefit other (non-Views) cases that need to call $item->view() repeatedly on the same field with different values.

Actual review now (nitpicks)

- The $viewDisplays member should be documented. Also, given that it only contains the custom runtime Displays created for the case of "->viewField(explicit display settings)", maybe it could be more accurately named customDisplays ?

- For code organization / symmetry, I'd favor a protected helper for the whole "figure out a $display for those $field_name and display_options" part of viewField(), that internally takes care of "is $display_options a $view_mode or an array of explicit settings".

- The piece of comment about the internal cache of displays currently in the phpdoc for the the helper would move inside the "if (array of explicit settings)" part of the code ?

Refactored as per #2.
+ renamed the property so that it's clearer that those persisted displays are only used for the case of "single field display" (which is not the main use case for EntityViewBuilder)

Side note - @msonnabaum : #2221577: Fix assumption that field settings is not a nested array also had to serialize+hash a settings array to use as a static cache key.

Over there, this was done with Crypt::hashBase64(serialize($settings)).
Patch here does crc32(serialize($display_options)

Should we unify ?

1. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -61,6 +61,15 @@ class EntityViewBuilder extends EntityHandlerBase implements EntityHandlerInterf
   +   * @see \Drupal\Core\Entity\EntityViewBuilder::getSingleFieldDisplay()
   +   *
   +   * @param \Drupal\Core\Entity\Display\EntityViewDisplayInterface[]
   

   @see should go below @param.. I think.

2. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +   * Returns the Display to use for rendering an individual field with viewField().
   

   As you argued in a different issue, some other subsystem could use the word Display, so maybe EntityDisplay would be better here. Or EntityViewDisplay? Also, 80 chars :)

3. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +   * @param string|array $display_options
   +   *   The display options passed to the viewField() method.
   ...
   +  protected function getSingleFieldDisplay($entity, $field_name, array $display_options) {
   

   How can this be a string when we type hint it as an array in the method signature? :P

4. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +      // Array of custom display options: use a runtime Display for the
   

   Same as above.

5. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +      // series of fields individually for cases such as views tables
   

   Missing a full stop.

6. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +      $entity_type = $entity->getEntityTypeId();
   

   We call this variable $entity_type_id these days.

7. +++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
   @@ -451,4 +439,52 @@ public function viewFieldItem(FieldItemInterface $item, $display = array()) {
   +        $this->singleFieldDisplays[$key] = entity_create('entity_view_display', array(
   

   All entity classes can use the ::create() static method now.

Thanks @amateescu :-)

1. In function phpdocs, yes, but this looks really weird in class members phpdocs ? Left it untouched for now.
2. Fixed
3. Fixed. Forgot to change the typehint for @msonnabaum's version, which was only about "array of custom settings"; Should hopefully fix the fails ?
4. The typehint was wrong, but that comment is correct : this is the part of the code that deals with "$display_options passed as an array".
5. Fixed
6. Fixed
7. This is just moved around, but sure.

No real reason for that $view_mode variable.

Pending an answer to #4, this looks ready to me.

Thanks @msonnabaum. I'll report that back in #2221577: Fix assumption that field settings is not a nested array

Then, RTBC, as per #9 and #10 ?

Yep, sorry I forgot to do it myself :/

Yeah I only used sha256 in #2221577: Fix assumption that field settings is not a nested array to not having any discussion about hashing. I completely agree with the statement

I think it's generally a bad habit to not distinguish between cryptographic and non/cryptographic hash requirements.

This issue is a major task that will improve performance significantly and the disruption it introduces is limited. Per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?, this is a good change to complete during the Drupal 8 beta phase. Committed fa1b5ba and pushed to 8.0.x. Thanks!

Yeah I only used sha256 in #2221577: Fix assumption that field settings is not a nested array to not having any discussion about hashing. I completely agree with the statement

I think it's generally a bad habit to not distinguish between cryptographic and non/cryptographic hash requirements.

FYI : changed Crypt::hashBase64() to crc32() in TDM::getPropertyInstance() in the cleanup patch in #2364267: Clarify the logic in TypedDataManager::getPropertyInstance()