Choose the autorelease pool implementation once - #410
Conversation
initAutorelease guarded its work with a test of AutoreleasePool, which stays Nil in a program that has no NSAutoreleasePool. The search was therefore repeated on every objc_autoreleasePoolPush and on every objc_autorelease of a fast ARC object, and objc_getClass hashes a string and walks the class table. Callgrind over a loop of push and pop in a program with no Foundation gives 516 instructions per pair, 65 percent of them in that lookup. Timed inside the process, ns per push and pop pair: 27.53 against 5.17 with no Foundation, and 5.17 either way with it, where the class is found on the first call and the guard closes. The implementation is chosen once rather than on the first call that finds the class. A pool is pushed and popped through whichever implementation is in force, so a program that pushed an ARC pool and then loaded Foundation would pop through NSAutoreleasePool a pool the ARC path had pushed. The suite passes, 198 of 198.
| static BOOL chosen; | ||
| if (!chosen) |
There was a problem hiding this comment.
I don't think I'd expect this to be faster. Checking a boolean and checking against nil compile to the same instruction on most architectures.
You might have some luck sticking a __builtin_expect here to make sure that the compiler knows that this is a slow path.
There was a problem hiding this comment.
You are right that the two guards compile to the same thing. Master is cmpq $0, AutoreleasePool / je / ret, this is cmpb $0, chosen / jne / ret.
The difference is not the compare, it is that master's guard never closes in a program with no NSAutoreleasePool. AutoreleasePool stays Nil, so every push and every autorelease of a fast ARC object falls through and calls objc_getClass again.
Callgrind over push and pop with no Foundation: 507.5 instructions per pair on master, 70 percent of the program's instructions in objc_getClass and the class table lookup under it.
With the lookup done once, 142.6 per pair. Wall clock 27.8 ns against 5.16.
I built the __builtin_expect version. It executes the same instruction count, 1.0151 billion either way for the same run, differing by about 20 in a billion, and 27.2 ns against 27.8 where two control runs of master in the same rounds gave 27.4 and 27.5. It moves the cold block, but the call is still on the path that is always taken.
There was a problem hiding this comment.
That was intentional, because it’s possible for this code path to be hit the first time before NSAutoreleasePool has been loaded. It’s less likely with the v2 ABI, but happened in some +load methods with the v1 ABI.
What is the use case for running without an NSAutoreleasePool implementation after the program has loaded?
We could move this initialisation into the class loader: when you load a new class, see if it’s NSAutoreleasePool and set this variable if so?
There was a problem hiding this comment.
Fair objection. Choosing at first use fixes the choice for the process, so an
early +load would lock out a Foundation loaded after it. That is worse than
the current setup.
The only use case I can honestly think of is ARC code linked against libobjc2 with no Foundation
which is not a typical use case. The guard only closes once the lookup succeeds,
so there it never closes: 507.5 instructions per push and pop pair against 142.6,
27.80 ns against 5.16. With Foundation it closes on the first call and the two are identical.
The class loader version is built: same numbers, 198 of 198, and it keeps the
recovery. I didn't put this version forward for a reason:
objc_autoreleasePoolPop reads the variable without calling initAutorelease, so
a pool pushed before the switch is popped after it.
Where NSAutoreleasePool has no -_ARCCompatibleAutoreleasePool,
that pointer reaches -release of a class which did not create it, nothing drains,
and with real ivars it aborts in free(). Class registration narrows that window rather
than closing it.
So the switch is needed for later pools and wrong for any pool straddling one.
If the token from objc_autoreleasePoolPush recorded which implementation made
it, the switch would be safe whenever it came. Is there a reason it cannot?
There was a problem hiding this comment.
Answering my own question: It can. The token is either an interior pointer into the ARC pool page, an
array of id, or an NSAutoreleasePool instance, which starts with an isa. Both
are pointer aligned, so the low bit is free. Push sets it on the ARC token and
objc_autoreleasePoolPop routes on the token rather than the variable.
Class registration and the tag together, on 403df0d:
| variant | instructions per pair | ns, no Foundation | late switch | straddling pool |
|---|---|---|---|---|
| master | 507.5 | 27.35 | kept | aborts, 134 |
| this PR | 142.6 | 5.15 | lost | drains |
| class loader | 142.6 | 5.15 | kept | aborts, 134 |
| loader and tag | 144.6 | 5.15 | kept | drains |
198 of 198 with the tag, both on the class loader version and on master. The
tag costs 2 instructions per pair either way and nothing measurable in time.
The switch still happens with the tag: the next push goes through a Foundation
loaded late, and the earlier pool still pops through the ARC path.
Only run on Linux with clang so far.
I can replace the commit with the last row.
initAutorelease guarded its work with a test of AutoreleasePool, which stays Nil in a program that has no NSAutoreleasePool. The search was therefore repeated on every objc_autoreleasePoolPush and on every objc_autorelease of a fast ARC object, and objc_getClass hashes a string and walks the class table.
Callgrind over a loop of push and pop with no Foundation gives 516 instructions per pair, 65 percent of them in that lookup. Timed inside the process, ns per push and pop pair: 27.53 against 5.17 with no Foundation, and 5.17 either way with it, where the class is found on the first call and the guard closes. The suite passes, 198 of 198.
The implementation is now chosen once. Before this, a program that pushed an ARC pool and then loaded Foundation would switch, and pop through NSAutoreleasePool a pool the ARC path had pushed. Draft because that is a change in behaviour, and the answer may instead be that the late switch is worth keeping and only the lookup should be cached.