Problem/Motivation

Plugin discovery requires file directory scanning, loading classes into memory, then reflection to check for attributes.

We have to load the class into memory before we can check for plugin attributes, and this memory then can't be freed again.

Core has more than 100 classes that are in plugin directories, but are not plugins, or are different plugins.

For example

Convert.php
CreateNew.php
Crop.php
Desaturate.php
GDImageToolkitOperationBase.php
Resize.php
Rotate.php
ScaleAndCrop.php
Scale.php

These are ImageToolkitOperation operation plugins, which are in core/modules/system/src/Plugin/ImageToolkit/Operation/gd

But because they're in core/modules/system/src/Plugin/ImageToolkit they get parsed in case they're ImageToolKit plugins, this loads all the classes into memory and requires FileCache to store 'null' entries (which include mtime etc. in reality) in APCu.

If we move the GD toolkit operations into a parallel instead of nested directory, then we wouldn't be parsing them twice (or at all, when we only get the toolkits and not the operations).

There are various other cases, some base classes which will be annoying but probably worthwhile to move, some helper classes in Views it looks like.

Plugin discovery has explicit logic for when it discovers non-plugins so we can add an Exception there to start building a list.

Steps to reproduce

Proposed resolution

Move base classes/interfaces to src/Plugin
Move plugins to their own 'type' directory, src/Plugin/my_plugin_type/plugin_name.php

Remaining tasks

Move classes around
Update Plugin API documentataion

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Comments

catch created an issue. See original summary.

joachim’s picture

We should define a standard for where plugin base classes and interfaces go.

Having these in the plugin folder is good DX. Having them dumped in top-level /src is messy.

I think @catch suggested /src/Plugin in slack?

catch’s picture

Yes I was thinking of moving them up one directory level into src/Plugin - keeps them close to the actual plugins, but not actually mixed in together then.

quietone’s picture

Issue summary: View changes

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

godotislate’s picture

Thinking about this some more: if there are interfaces, traits, or base classes in a plugin directory, and if any plugin class in the same directory uses that interface/trait/base class, the performance gain might be marginal, since they would be loaded into memory anyway. They would take up additional entries in the file cache, though.

joachim’s picture

> They would take up additional entries in the file cache, though.

Could we mitigate that by having discovery simply not look inside any file called *Trait, *Base, or *Interface? Don't even scan for an attribute, just skip it in the file finder stage.

I do like the DX of those classes being in the same folder as the plugins!

catch’s picture

That would work except for the specific case of #3502584: [PP-1] Remove nesting of ImageToolkitOperation plugins but that's pretty unique.

We have a coding standard that enforces that naming convention now, so it should be pretty reliable #3521443: Traits should always have the suffix "Trait".

godotislate’s picture

Could we mitigate that by having discovery simply not look inside any file called *Trait, *Base, or *Interface?

We have coding standard rules for Interface and Trait, but not Base. Is it completely safe to skip all classes that end in Base? Not sure.

catch’s picture

There's an issue here: #1567920: Naming standard for abstract/base classes but it's a bit stalled, and it at least partly got stalled on whether all base classes should be abstract which would involve code changes.

We could skip interfaces and traits for now, and then open an issue to skip base classes postponed on that issue?