I think buildQuery() is just called like that because that's also how the method is called in the database storage controller, it's called from load().

Wondering about that, why do we need this in the first place? Maybe it was necessary when we had to support $conditions but now, we can simply loop over the id's and do a config($prefix . $id) on them, no?. @yched suggested in #1880766: Preload configuration objects that we'd add support for loading multiple config files at once, mostly because we could then also do a getMultiple() against the config storage cache.

Added debug($this->entityType); in that if and got dozens of 'editor''s and a bunch of 'field_config''s.

Looks like we need support for a fullyLoaded flag so that we don't run through this multiple times. Also, we could combine that with a id map and load all at once from the cache or something like that.

The culprit in the Editor module is editor_load(), which look like a premature optimization anyway.

For the field module, those calls should be cached already (in the field_info:instances cache), not sure why it's not working for you.

The main problem is that the storage structure is really stupid. Browsing files like that from the filesystem cannot possibly scale. We need to either start hashing this directory or move the store to a proper database (like SQLite or a db* file).

Yeah, the problem with it is that it assumes that the entity_load_multiple('editor') call is somehow cached and initially faster, which isn't the case.. The optimization makes sense I think because it seems to be called for all formats of a textfield, so if it's called, then it really is called for all of them. If we can improve the listAll() and do a config getMultiple() then this would be true.

The problem with fields seems to be that it assumes that there is a call to field_info_instances($entity, $bundle), where it would pre-fill the internal field by id static cache. That call never seems to happen, possibly due to the EntityNG conversion of nodes so each field is loaded separately.

quicksketch and I worked on editor.module, he wrote editor_load() specifically; I talked to him and he's fine with changing that to not doing the multiple loading, especially now that anonymous and authenticated users now have one format anyway.

I don't mind writing that patch, but berdir is saying we can (should?) fix listAll() and introduce $configStorage->getMultiple() to make it faster.

So… which should we do? Fix editor+field or improve "all entities" loading?

I think those are three different issues. This issue should attempt to make the load all use case as fast as possible, which is currently used quite often for config entities (that's how EFQ works, for example).

Once this is fixed then the editor_load() issue depends on what we expect in terms of actually using/seeing all editors for most users or not and how much memory overhead this would mean. If we try to be intelligent, we could also load them based on the editors a user would be allowed to see?

And the field case sounds like yet another separate bug. There is specific code to avoid those fields being loaded separately and that doesn't work right now.

Regarding fields & FieldInfo cache :
My hope/target is to be able to ditch FieldInfo's persistent cache of Field and FieldInstance config entities, and rely solely on the config() cache on raw config data. Caching ConfigEntities on top of it sounds like a recipe for hair loss.
The "static" cache in FieldInfo would stay of course, this would be where Field and Fieldinstance objects are persisted across the request once they've been loaded (and we usually need them at several places in the callstack within a request).

The flow would be:
- FieldInfo uses config_get_storage_names_with_prefix() (which currently does listAll() / glob()) to build a "field map" (which fields / instances are present on which entity types / bundles), and persistently caches it.
- FieldInfo::getBundleInstances() figures out a list of config names to load based on the "field map", multiple-loads the config items (hitting config persistent cache), and statically persists them.

This relies on being able to multiple-load config items, and on the assumption that instantiating a ConfigEntity from raw config data is not too expensive and does not need to be persistently cached...

Here's a proof of concept that introduces ConfigFactory::loadMultiple() and a listAll() cache. Seeing some nice performance improvements (700 instead of 800 queries) but I doubt it will get through the tests just yet ;)

Notes:
- loadMultiple() is a bit tricky because ConfigFactory/Config are specifically built to support lazy loading but here we actually don't want that and want to load the upfront. Note that lazyloading is pointless for config entities anyway as we do a get() right after loading them anyway, so this is no no way a regression.
- Unlike get(), loadMultiple() only returns existing, successfully loaded configuration objects
- Note sure about listAll() being a separate or a single cache entry. A single one might make sense as we can do more intelligent cache clears and only have to do a single cache get for them.

While this issue has derailed a bit from its title, I think we can still have a really quick win for cold caches by replacing glob() with GlobIterator, at least until someone takes on the last paragraph from #5 :/

I got a nice ~5ms improvement on a test site with 600 config files, and ~10ms with 1200 files, basically for free.

The reason we're doing a isNew() check here isn't just because this could theoretically create a new config, it's also because deleted configuration is kept in CacheFactory::$cache as a new config object, as that's what Config::delete() is doing.

I just moved that check one level down as it's a problem within the config system and not config entities. Seems like deleted configuration should get removed from there.

During installation, we're re-creating the container many times. As we're using the memory backend cache, each time we get a new CachedStorage(), we also get a new MemoryBackend, with a separate storage. Which means that in same cases, we call deleteTags() on one memory backend and load from another that still has the cache, which leads to inconsistent results.

The config factory has the persisted tag while the cached storage doesn't. So direct access to the storage will result in a different instance being used. So it seems only obvious to make this persisted as well.

We still don't have a solution for inheritdoc + additional stuff, that said, this actually seems wrong (I copied it). Seems to me that a class that throws exceptions that are not defined in the interface violates that interface but that's not related to this issue.

After installing this patch and query cache_config table I noticed we have some duplicate keys due to a inconsistency with how we pass in the prefix for config entities eg. list:views.view. and list:views.view

Okay so lets not cache when a prefix is not provided. This is the first step in refactoring the storageinterface to have separate findAll() and findByPrefix() instead of a do-it-all listAll() method. But rather than do that here lets keep the scope creep down and do this is a followup.

I made it static because there were many different config storage objects floating around during installation and module enabling. Maybe that's no longer the case thanks to the persist tag, I need to check that.

Also had a quick look at doing a single cache, but it looks like there aren't that many different listAll() calls on the same page, so let's keep it separate for now.

What I did see was an insane amount of entity constructing for field config entities (again) as in 12k config entities insane ;). We'll need to look into that after this is in, I thought I fixed it with the field definition cache, but something changed it again it seems...

Ok, this turned out to be a tag checksum static cache problem. When we delete a tag multiple times on page requests, then the parent (test) process is not being updated and still uses the deletions count from the static cache. Very surprised that this does not cause failures anywhere else.

I had to read this three times, because I initially read it as "when items are successfully loaded, the cache backend removes them from the cache". Could we just say "Items loaded from cache are removed from the list of $names to fetch" or similar?

Should the cache tag be configFindByPrefix? or 'configPrefixes'? Not sure it should but the tag name jumped out a little given it's a verb - almost looks like it's an options array to deleteTags() to find the tags by prefix (which would be horrifying).

So #2024833: Adopt load() and loadMultiple() on entity storage controllers changed a bunch of things but missed out changing the mock in \Drupal\user\Tests\Views\Argument\RolesRidTest this currently works in head because the call to loadMultiple \Drupal\user\Plugin\views\argument\RolesRid::title_query() just depends on the filesystem and entity loading ... this patch introduces a dependency on the cache system.

That won't actually break anything at the moment, because the tag is only used for one bin, but it's wrong API usage (which shows we should do the split). I think we could commit the one-liner here so re-opening.