Make TypedData implement PluginInspectionInterface

+++ b/core/lib/Drupal/Core/TypedData/TypedData.phpundefined
@@ -7,13 +7,16 @@
+use Drupal\Component\Plugin\PluginInspectionInterface;
...
+abstract class TypedData extends PluginBase implements TypedDataInterface, PluginInspectionInterface {

This is redundant as Drupal\Component\Plugin\PluginBase already implements PluginInspectionInterface, but I guess it can't hurt.

+++ b/core/lib/Drupal/Core/TypedData/TypedData.php
@@ -41,6 +43,10 @@
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param array $plugin_definition

I must say I'm not in favour of having those generic plugin_id and definition in there. That only makes sense to you when you are exactly clear which plugin this is about, but when someone is working with those classes that's not necessarily the case. (So, yes I can follow the reasoning people don't like generic typed data stuff in their classes - it's the same.)

Cannot we just call it differently here, e.g. $data_type and $type_definition? Or is having $plugin_id & $plugin_definition everywhere the common pattern?

In general, that names would work for me. But then, I just realized $data_type is now duplicating $definition['type'].

However, I just realized that this will add two additional object properties to each typed data object, which results in additional memory usage. I don't recall the numbers any more, but I think it was significant :/ A quick check on a patch with loads of entities (300 comments?) would be a good idea I guess.

So, instead of doing that I guess we should come up with an optimized implementation. plugin-id? -> that's equal to getType(). plugin-definition - couldn't we look that up via the manager? We'll have to inject it anyway, as other methods use it already.

So here's the alternative that I think was suggested in #9. Given that TypedData is already accessing the manager in that way in its other methods, I think this is ok for now. However, like yched, I prefer #11 if fago is ok with it.

I got different numbers than #13 though for a node with ~250 comments viewed as root user. Each run I had to refresh about 3 times before the memory numbers stopped varying:

HEAD:
Page execution time was 5491.79 ms. Memory used at: devel_boot()=4.06 MB, devel_shutdown()=23.68 MB, PHP peak=32 MB.

#11:
Page execution time was 5567.52 ms. Memory used at: devel_boot()=4.07 MB, devel_shutdown()=23.79 MB, PHP peak=32.25 MB.

#14:
Page execution time was 5492.87 ms. Memory used at: devel_boot()=4.07 MB, devel_shutdown()=23.69 MB, PHP peak=32 MB.

So, #14 is a little more performant, but probably only because getPluginDefinition() isn't being called on it yet. That can change with #1969728: Implement Field API "field types" as TypedData Plugins.

In short, I'd be fine with whichever one gets RTBC'd that helps #1969728: Implement Field API "field types" as TypedData Plugins most.

#11 has some unnecessary whitespace:

+++ b/core/lib/Drupal/Core/TypedData/TypedData.php
@@ -64,6 +76,20 @@ public function getType() {
+  }
+  ¶
+  /**

Very interesting comparisons! It's interesting that the results of effulgentsia are better. Maybe, did you use PHP 5.4 vs yched (and me) PHP 5.3 ?

The definition arrays are not physically copied, PHP's copy-on-write optimizes that (and those are never written to, just read from).

I know - still, it's just the additional object property that results in more memory being used - its content doesn't matter.

So, #14 is a little more performant, but probably only because getPluginDefinition() isn't being called on it yet. That can change with #1969728: Implement Field API "field types" as TypedData Plugins.

I guess that's also because of less stuff being assigned - this code runs through very often. Also, once the manager becomes injected, I don't see a reason to repeat the type definition in the class. We already have problems like #1875182: Pass plugin definition to plugin instances instead of discovery object - but we'll have to solve it at least for the typed data and entity managers in order to inject dependencies into fields and entities anyway.

So - given #14 is simple, faster and uses less memory and we really have to watch performance I think we should go with this one. If field types start using getPluginDefinition() very often, we can still optimize it more.
Thus, I'd call #14 RTBC - if that works for you?

Yay. Glad there's agreement.

It's interesting that the results of effulgentsia are better. Maybe, did you use PHP 5.4 vs yched (and me) PHP 5.3 ?

Yes, my numbers are for PHP 5.4.10.

Committed 94b0b70 and pushed to 8.x. Thanks!