[fpc-devel] Patch for rtl/inc/rtti.inc

[Daniël Mantione]

Op Sun, 14 Sep 2008, schreef Jonas Maebe:

>
> On 14 Sep 2008, at 15:11, Daniël Mantione wrote:
>
>> Op Sun, 14 Sep 2008, schreef Jonas Maebe:
>> 
>>> I don't think it's a good idea to start manually adding loop induction 
>>> variables to many routines in the rtl. It makes the code less 
>>> maintainable, and at one point the compilation process itself will/should 
>>> do that by itself.
>> 
>> This patch should get applied. It's debatable wether additions are hard to 
>> maintain than multiplications
>
> Having one or two variables that need manual updating in a loop vs none at 
> all reduces maintainability. It's one of the reasons why people use for-loops 
> rather than while-loops.

While I don't disagree, it's debatable, because it solves another 
maintainability issue: A simple procedure call is more maintainable than 
one with all kinds of expressions inside.

>> and it may take years before the compiler can do that optimization. That 
>> doesn't mean we should optimize anything manually, but fpc_copy is 
>> definately speed critical code for things like widestrings.
>
> Almost every loop in the RTL is criticial for at least one language feature 
> which is heavily used by a certain category of programs.

You are completely correct here, this is why a lot of effort goes into 
optimizing the system unit, it is one of the most speed critical hotspots.

Anyway, this should be viewed from an engineering point of view: 
Maintainability is no goal by itself, nor is speed, good engineering 
balances both requirement. Recklessly rejecting patches is wrong, because 
someone may have important interrests in having that code fast. The right 
approach, if you are concerned about maintenance is ask the user to show 
speed has to have more interrests here, for example ask the user to show a 
real-world situation where these routines show up high in a profile. But 
we don't need to here, since we can see the speed interrests here 
ourselves and the maintainability issues are minor.

Daniël