Hi Levente.

> reorder the clauses based on the frequency of the positive matches

I think we should estimate the frequency of the "search term" not the positive matches. I think it is more likely that one checks for #> or #+ rather than 'true'.

Anyway, the false-case should be as fast as possible because it is the most common one given the overall value range of literals.

Best,
Marcel
Am 24.07.2019 13:51:27 schrieb Levente Uzonyi <leves at caesar.elte.hu>:
Hi Marcel,

On Wed, 24 Jul 2019, commits at source.squeak.org wrote:

> Marcel Taeumel uploaded a new version of Compiler to project The Trunk:
> http://source.squeak.org/trunk/Compiler-mt.405.mcz
>
> ==================== Summary ====================
>
> Name: Compiler-mt.405
> Author: mt
> Time: 24 July 2019, 11:17:04.167109 am
> UUID: 7354b36e-2509-4f23-b110-1b9cfe0310d2
> Ancestors: Compiler-mt.404
>
> Adds message to avoid costly byte-code scanning for has-literal checks.
>
> Note that ((... or: [...]) or: [...] ...) is a little bit faster to evaluate to false than (... or: [ ... or: [ ... ] ] ). I would suspect not but deeply nested blocks seem to have an interesting effect here.

I don't think there's any real difference between the two's performance.
If you have a look at the generated byte codes, you'll find that the
Compiler has inlined the blocks in both cases.
So, I preferer the latter version, because of better legibility.

There are at least three possibilities to speed up the method:
- reorder the clauses based on the frequency of the positive matches
- use the JIT's capability of generating better machine code when a
variable compared with a constant is the receiver of a "boolean message"
- use the JIT's capability of generating better machine code when
the returned value is a known boolean

Based on those, the following implementation ought to be faster:

aLiteral == true ifTrue: [ ^true ].
aLiteral == false ifTrue: [ ^true ].
aLiteral == nil ifTrue: [ ^true ].
(aLiteral isInteger and: [ aLiteral between: -32768 and: 32767 ]) ifTrue: [ ^true ].
(aLiteral isCharacter and: [ aLiteral asInteger <= 65535="" ])="" iftrue:="" [="" ^true="">
(aLiteral isSymbol and: [ Smalltalk specialSelectors includes: aLiteral ]) ifTrue: [ ^true ].
^false

Levente

>
> =============== Diff against Compiler-mt.404 ===============
>
> Item was added:
> + ----- Method: BytecodeEncoder class>>canBeSpecialLiteral: (in category 'testing') -----
> + canBeSpecialLiteral: aLiteral
> + "This check can be used to prevent unnecessary use of #scanBlockOrNilForLiteral:. For performance, this method summarizes specializations from all known bytecode encoders. It is not meant to be refined per encoder."
> +
> + ^ ((((((aLiteral isSymbol and: [Smalltalk specialSelectors includes: aLiteral])
> + or: [aLiteral isInteger and: [aLiteral between: -32768 and: 32767]])
> + or: [aLiteral isCharacter and: [aLiteral asInteger <=>
> + or: [aLiteral == true])
> + or: [aLiteral == false])
> + or: [aLiteral == nil])!

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20190724/9fe19a67/attachment.html>