Always use infoParser in ExtensionDiscovery

This removes the $use_info_parser = FALSE path and makes InfoParser mandatory, so ExtensionDiscovery::scan() now always parses the complete .info.yml file instead of using the previous lightweight type: lookup. Could we add or extend a test covering the discovery behavior that previously used $use_info_parser = FALSE, to make sure there isn't an unintended behavioral or performance regression?

Also, since the ExtensionDiscovery constructor signature is changing, this looks like a public API change. Could we confirm whether a change record is needed for this?

Constructors are considered @internal but we should add a change record anyway probably.

We're going to need to consider the performance ramifications of this change. I'm pretty sure this exists to stop thousands of yaml reads during install.

We do a lot of YAML info parsing during install already, per #3616275: Use the PECL YAML parser for info file parsing when available although yes it would be good to check how much, if any, this adds on top of that.

But last time I looked at this (some time ago around #3493423: Deprecate the $use_info_parser parameter to ExtensionParser::scanDirectory()) we were often doing the custom parsing once and then parsing the YAML again in the same request.

On a minimum install using Drush

HEAD

*** SPX Report ***

Global stats:

  Called functions    :     5.2M
  Distinct functions  :     7.5K

  Wall time           :    2.41s
  ZE memory usage     :   70.2MB

MR

*** SPX Report ***

Global stats:

  Called functions    :     5.5M
  Distinct functions  :     7.5K

  Wall time           :    2.49s
  ZE memory usage     :   72.1MB

So we're using more function calls and and more memory - what is the motivation behind this change other than less options in the constructor?

If we add this MR plus the pecl yaml together we get

*** SPX Report ***

Global stats:

  Called functions    :     5.0M
  Distinct functions  :     7.5K

  Wall time           :    2.38s
  ZE memory usage     :   72.1MB

So less function calls - which makes sense - but still the increased memory usage...

Did some quick profiling.

I hacked GenericTestBase to do an early return, and then node module's GenericTest with HEAD vs. the MR with xhprof enabled. The MR adds only two calls to Yaml::decode()

1324 in HEAD and 1326 with the MR.

The different profiling results are interesting. I'm wondering how much this is the real installer vs running a functional test, or whether drush is also doing some extension parsing itself to find plugins etc. Once way to find out would be to compare drush to dr install I guess (or look at the specifics of what's calling this from drush).

Note there's also #3303751: Reduce the number of times UpdateRegistry scans for extensions which is postponed on this issue.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.