Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Kernel listeners are registered twice, which slows down things quite a bit as all listeners are called twice (and he might even have bad and hard to find side-effects).
Proposed resolution
This patch removes the duplication. Enjoy the performance improvement!
Comment | File | Size | Author |
---|---|---|---|
free-perf-improvment.patch | 935 bytes | fabpot | |
Comments
Comment #1
dawehnerThank you!
This issue was mentioned before, #1873436: Compiler compiles everything twice and indeed your patch fixes this problem.
I'm wondering whether this should be put into a test.
Comment #2
kscheirerWow, good catch!
I think this does deserve a test, since this fixes behavior that should not have been broken in the first place. The test would be to register a listener, trigger the event, and then check that the listener is only executed once.
It's a clear performance gain I'm sure, but some kind of benchmark will go a long way to convincing core maintainers to get this patch in fast.
Comment #3
fabpot CreditAttribution: fabpot commentedI will let you work on the tests and benchmarks. I don't see the point of those as this is clearly a typo.
Comment #4
marcingy CreditAttribution: marcingy commentedThis is just removing duplicate code. So the fact that tests still pass means it has tests.
Current code is
Comment #5
Crell CreditAttribution: Crell commentedThat's weird, as I'm not sure I ever saw listeners firing twice. Some of them would have caused fatal errors if they ran twice.
Comment #6
Crell CreditAttribution: Crell commentedCrosspost.
Comment #7
Crell CreditAttribution: Crell commentedNow that I look at the actual code, not just the patch, this is a major "Duh". :-) +1 on RTBC. I don't think this needs a test or benchmark, as even without benchmarks the current code is obviously wrong.
Comment #8
msonnabaum CreditAttribution: msonnabaum commentedI agree that a benchmark is unnecessary. This seems more like correcting a mistake that has the side effect of a performance improvement. Best to just correct the mistake.
Comment #9
catchCommitted/pushed to 8.x, thanks!