Add transaction-related events to the Database API

For inspiration see https://github.com/laravel/framework/tree/10.x/src/Illuminate/Database/E...

eesh, seems not-ideal to wait until most-everything is destroyed to commit transactions, i.e. difficult to respond to various transaction commit failure modes.

I did my best to review the MR. Unfortunately, I don't know enough about the database system to review it well from a technical perspective. My comments on the MR amount almost entirely to nitpicks on documentation. I'm sorry about that. For what it's worth, I did read through all the code change and it looks good to me.

I nearly forgot: since there are three new methods on TransactionManagerInterface we'll need a change record, won't we?

Just curious why these are not hooks?

Dispatching an event is faster than invoking a hook (and uses less memory), so it doesn't make sense for low level code like the database layer to invoke extra hooks, especially in the context of performance analysis.

And I wouldn't mind having some new events for instrumenting how slow the hook system is 🙃

Dispatching an event is faster than invoking a hook (and uses less memory), so it doesn't make sense for low level code like the database layer to invoke extra hooks, especially in the context of performance analysis.

Do you have a source for that? I don't see how that is possible.

We just decoupled hooks from events because of the performance problems. #3506930: Separate hooks from events

Event Dispatcher does not scale well, I think a hook here is better served.

Do you have a source for that? I don't see how that is possible.

In my testing, Drupal::moduleHandler()->invokeAll() is slower than Drupal::service('event_dispatcher')->dispatch()

Both the first call, and the second call are slower.

Events are of course heavily used in low-level code, for example, request events/event subscribers. So that seems like the obvious building block we'd use to allow for instrumenting various low-level subsystems. And we already are using as mondrake mentioned.

Sorry to keep belaboring this:

In my testing, Drupal::moduleHandler()->invokeAll() is slower than Drupal::service('event_dispatcher')->dispatch()

Which hook did you invoke, how many implementations were there, what did that hook do?
Which event did you dispatch, how many events were implemented, and what did those events do?

Again I'm asking because this does not align at all with the numbers we saw. Removing event dispatcher from hooks showed the following improvements courtesy of @berdir:

Time: 15% (5.69 ms → 4.82 ms)
IO: 18% (2.05 ms → 1.69 ms)
CPU: 14% (3.64 ms → 3.13 ms)
peak memory: 23% (2.68 MB → 2.07 MB)
network transfer: 29% (0.914 MB → 652 kB)

If you share how you tested this I can try to replicate your findings.

I'm not sure how to dynamically register a hook implementation so that we can make a nice little test script, but we can add an event listener and dispatch an event, and then in comparison, invoke a hook that doesn't exist:

$bytes = memory_get_usage();
$nanoseconds = hrtime(TRUE);
Drupal::service('event_dispatcher')->addListener('my event', fn () => print 'EVENT: ' . PHP_EOL);
Drupal::service('event_dispatcher')->dispatch(new stdClass(), 'my event');
echo (hrtime(TRUE) - $nanoseconds) / 1000, ' microseconds', PHP_EOL;
echo memory_get_usage() - $bytes, ' bytes', PHP_EOL;

echo 'HOOK: ', PHP_EOL;
$bytes = memory_get_usage();
$nanoseconds = hrtime(TRUE);
Drupal::moduleHandler()->invokeAll('not actually a hook');
echo (hrtime(TRUE) - $nanoseconds) / 1000, ' microseconds', PHP_EOL;
echo memory_get_usage() - $bytes, ' bytes', PHP_EOL;

Looks like moduleHandler()->invokeAll() can make database queries - cache gets - which I can't imagine you'd want to do as part of dispatching events inside the database layer.

This is a very large MR, difficult to review I'd imagine (not sure I'm one to do that) hence leaving in review. But wanted to ask if could be broken up?