[Mono-devel-list] [PATCH] Faster String.Equals

[Paolo Molaro]

On 05/22/04 Andreas Nahr wrote:
> However I see some problems with that approach:

Note, most (all?) of these issues were considered when I proposed this
optimization (bug#58026).

> 1. Substantial code multiplication. We need Supported_Architectures + 1 code
> implementations for every method instread of just 1. This will be horrible
> to maintain and a lot of additional work.

Nope: they are optional, so architectures don't need to write code for
them.

> 2. Too platform specific optimizations. Look at your String.Equals that is
> only implemented for x86. If somebody uses a lot of String.Equals in his own
> classes then his implementation will perform seriously slower/ different on
> other platforms. This may lead to problematic optimizations for classes
> which internally use these methods.

These kind of assembly-coded icalls will be used for three kinds of
methods:
*) code which cannot be expressed as IL/C# code
*) code which would require a managed->unmanaged transition
*) code that the compiler won't be able to optimally compile, just like
some functions in libc are coded in asm and not in C (also code which
make take advantage of features in the current processor)

> 4. Although it does not have the overhead of an icall AFAIU it still poses a
> problem for optimizations. A compiler (like ngen) may be able to completely
> eliminate a case like:
> "Test" != "Test"
> to a false constant (very simple example). Is that still possible with your
> implementation?
> 
> Or
> String a = "dh"
> String b = "dfh"
> a.Equals (b)
> might get inlined and *optimized* by the compiler or JIT or especially AOT
> because it can guarantee that a and b can never be null (which is checked in
> Equals).

Inlining can still happen just fine.

> 6. We still need a fast managed implementation for cases when a ASM -
> version is not available

We need it also when it's present, so the inliner can see the code and
take advantage of it. This is one of the reasons Ben's patch is not
going in: we first need to optimize the current C# code and the code
produced by the JIT for those methods (the current String.Equals
generated code could be trivially made better), since those changes
benefit everyone and having it in critical methods should just be
considered an incentive to make it better:-).
The other reason is that his code is still slow and it's not worth the 
complexity and the uglyness of adding asm-based icalls if the code 
is still not optimized.

> 1. Add that ASM - injection architecture, but not enable it by default. We
> could have a --AsmInject switch which enables it.

Yes, we could commit the stubs and add a separate optimization flag
which is not selected by -O=all.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better