filter_list_format() hits database too often / filter_format_save() doesn't save all filters

New patch, fixed tests, added caching for the query, avoids doing the extra work on the filters up front in case they don't actually get used.

HEAD:
Executed 72 queries in 45.77 milliseconds.

Patch:
Executed 70 queries in 46.68 milliseconds

So, we reduced the number of queries, but we actually got slower?

@Dries, devel query timings has something like a 50-100% standard deviation and isn't to be trusted unless it's dozens of queries or a huge filesort or something - page execution time is similarly unreliable, so those pastes are only for the query count (which is reliable as long as you have a warm cache).

It's also likely that benchmarks won't be conclusive with just these couple of queries, although I'm happy to run them.

The reason I post these patches is because:

* Less round trips to the database and less PHP overhead executing queries is worth having even when the queries themselves are small and indexed.
* 2-3 queries quickly adds up when it's multiplied across multiple issues and subsystems.

In this case, devel was accurate, because I was missing an index change on the {filter} table, leading to a 10ms query, whoops!

Thought of a benchmarking scenario, not the most realistic in the world but not impossible either.

I added a bunch of input formats so that were 15 enabled on the site, gave anonymous user access to 14, access to and view comments, then benchmarked node/1 ab -c1 -n1000

HEAD:
Requests per second: 9.83 [#/sec]
Executed 65 queries in 36.52 milliseconds.

With patch:
Requests per second: 10.51 [#/sec]
Executed 52 queries in 23.04 milliseconds.

I don't know any sites with 14 input formats, but I do know some with 6 or more. So that 7% change probably equates to 2-3% on a more realistic site.

We still need to resolve the admin/config/content/filter issue though per #7.

The last submitted patch failed testing.

The last submitted patch failed testing.

This patch nicely exposes a bug I already encountered in #394466-28: Full HTML description on node form is not descriptive enough, but didn't create an issue for yet.

filter_format_save() needs to save all available filters for a text format, regardless of their status.

This patch should hopefully fix it.

cross-posted. Merged in that index change.

omg. Either of those two should be fine :p

The last submitted patch failed testing.

ok, that was quite a challenge. I'm totally happy that we have *so extensive tests* for filter.module. Changing this code lead to subsequent different fails in filter tests, which is awesome.

oh nice, this also requires us to finally fix the installer. :)

Also fixing PHP module's raw data query insertions...

The last submitted patch failed testing.

The last submitted patch failed testing.

The last submitted patch failed testing.

erm, considering all the filter system and installation profile bugs that are fixed in this patch, bumping to critical.

Thanks, catch! You saved us from monster post-release headaches!

Re-rolled against HEAD.

Also, to summarize these changes:

• filter_list_format() needlessly executed one query per text format to retrieve all filters in it. It now performs only one query.
• filter_list_format() was needlessly invoked on the text format add form, where we can be sure that no filters exist in the database yet. It no longer is invoked now.
• That, however, lead to the problem that filter_list_format() returned default filter settings for all filters without configured settings before. Since it is no longer invoked during initial text format creation, filters in new text formats did not contain any settings. Hence, the merging of default filter settings was moved into filter_format_save().
• That, however, lead to the problem that system.install and php.install executed raw database queries to insert initial text formats and filters without using the Filter API, so the initially created text formats contained filters without any settings. This was changed so that this code properly uses the Filter API to create text formats. I already had this change on my todo list, and when doing so, I also wanted to move this code into installation profiles (because that's where it belongs), which is now included in this patch.
• That, however, triggered a yet invisible critical bug: Since module hooks are not weighed after module dependencies (only by module weight and filename), and php.module comes therefore before system.module, the "PHP code" text format was suddenly the default format in PHP module's tests. That is, because all text formats were saved with a weight of 0, so PHP's text format was inserted first, and the default installation profile simply assumed that text format ID 1 would be "Filtered HTML". Hence, anonymous users and authenticated users were granted the permission to use PHP code instead of Filtered HTML. Likewise, PHP module's tests as well as some other tests did the very same fragile assumption, so they tried to test evaluation of PHP code with the last text format id in the database, which was now "Plain text".

Looks fine here, Dries already reviewed, let's get this in before it reaches 40k ;)

Automatically closed -- issue fixed for 2 weeks with no activity.