Use ifNil: in isShorter:than:.

[fniephaus]

I'd argue this is slightly more idiomatic and it seems that this is also supported in SOM. In fact, the Squeak/Smalltalk compiler turns ifNil: into an identity check (== nil) plus jump rather than sending isNil to the receiver and then jump.

I noticed this some time ago when I looked into excessive splitting in TruffleSqueak (here's
a recent example). It turned out that the additional isNil send causes these splits.

Surely, there are ways to fix the splitting issue in TruffleSqueak. I just thought I open this anyway because, again, it is more idiomatic and because of that, also specifically optimized by the Squeak/Smalltalk compiler.

[smarr]

Looking at the other languages, I think the change should be fine.

Happy to merge it, but would prefer to keep the SOM and SOMns benchmarks in sync with the Smalltalk ones.

And yeah, the splitting issue, you probably still want to fix :)

[fniephaus]

Oh right, the SOM impl is separate. For some reason, I thought it’s derived from the Smalltalk code. I pushed another commit.

[eregon]

FWIW I recall that we did similar for Ruby but everywhere: 8ccc0c9 (for idiomatic code reasons mostly, .nil? avoids splitting on TruffleRuby).
With some follow-ups later to change e.g. if liter.parent == nil into unless liter.parent as suggested by RuboCop.

[eregon]

There are a few more isNil ifTrue:, probably it would be good to change them too:
https://github.com/search?q=repo%3Asmarr%2Fare-we-fast-yet%20%2FisNil%20ifTrue%3A%2F&type=code

[krono]

Note that some of these are newspeak, but the reasoning shouldn't be different.

[krono]

The reason of ifNil: not being used yet is that the original SOM, from which the benchmarks where pulled and built upon, did not support ifNil: back then.
Times are different now :)

[fniephaus]

There are a few more isNil ifTrue:, probably it would be good to change them too:
https://github.com/search?q=repo%3Asmarr%2Fare-we-fast-yet%20%2FisNil%20ifTrue%3A%2F&type=code

After looking at other usages, I'm not so sure anymore how far we want to go here. There are, for example, isNil ifTrue: [] ifFalse: [] usages, which I would change to ifNil: [] ifNotNil: [], but that's not supported in SOM. Then, there also are usages of notNil ifTrue: [], which should probably be changed to ifNotNil: [], which is supported in SOM.

Any thoughts?

[smarr]

Yeah, I pushed the SOMns commit, mostly to indicate that it's also separate code.
And, I didn't immediately merge it, because as @eregon pointed out, it would make things inconsistent. When looking at what we did for the other languages, it seemed desirable to apply this consistently to all benchmarks.

That some variations are currently missing from SOM seems only a small hurdle. Though, it's a bit of extra work. @OctaveLarose, for SOM-RS, did you implement special handling for Boolean>>#ifNil:*? I think I may have it for some of the SOMs already but not consistently.

[OctaveLarose]

som-rs doesn't have ifNil, no. But it looks like it could be a speedup in a few benchmarks, and I could easily implement it. A lot of the code takes inspiration from PySOM's design, and I don't think it has any primitives for it, either.

It sounds like a good change, though! I'd not realized it was a common case.

[smarr]

The update for SOM is here: SOM-st/SOM#123

[smarr]

Performance difference on the SOM implementations without specific support for ifNil:* variants: https://rebench.dev/Are-We-Fast-Yet/compare/2e1a50989356d8beb32c55393c8c2087d7f77707..a31e167bf1d6812a2251e10321d81abf0a3fab54

It looks a bit noisy, and there might be some extra overhead compared to the inlined isNil ifTrue:* version on List and Havlak.

With inlining of ifNil:* and variants, it seems a bit noisy and unclear, but no major change:
https://rebench.dev/Are-We-Fast-Yet/compare/2e1a50989356d8beb32c55393c8c2087d7f77707..807f8d81edef384d50638b82b80da6cb0f9e4775