Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2008-09

Re: Patch for rtl/inc/rtti.inc

8 messages · 4 people ·

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Jonas Maebe
to Markus Beth: this patch rewrites code of ArrayRTTI and fpc_Copy to allow the compiler to generate…
Markus Beth wrote 7 lines
this patch rewrites code of ArrayRTTI and fpc_Copy to allow the compiler to generate faster code (at least on i386). The change in ArrayRTTI yields a performance gain of ~4% for our real world application on a 3 GHz Intel Xeon. The change in fpc_Copy is completely untestet because I don't know when this compilerproc is called. Maybe someone who knows can hack a quick benchmark...

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.

Jonas

Reply by email

Markus Beth

Hi,

this patch rewrites code of ArrayRTTI and fpc_Copy to allow the compiler to generate faster code (at least on i386).
The change in ArrayRTTI yields a performance gain of ~4% for our real world application on a 3 GHz Intel Xeon.
The change in fpc_Copy is completely untestet because I don't know when this compilerproc is called. Maybe someone who knows can hack a quick benchmark...

Markus

P.S.: Do you prefer to get patches like this via bugtracker or via mailinglist?

Reply by email

Daniël Mantione
to Jonas Maebe: I don't think it's a good idea to start manually adding loop induction variables to many…

Op Sun, 14 Sep 2008, schreef Jonas Maebe:

Show quoted text 15 lines
On 14 Sep 2008, at 14:49, Markus Beth wrote: > this patch rewrites code of ArrayRTTI and fpc_Copy to allow the compiler to > generate faster code (at least on i386). > The change in ArrayRTTI yields a performance gain of ~4% for our real world > application on a 3 GHz Intel Xeon. > The change in fpc_Copy is completely untestet because I don't know when > this compilerproc is called. Maybe someone who knows can hack a quick > benchmark... 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 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.

Daniël

Reply by email

Florian Klaempfl
to Daniël Mantione
Show quoted text 25 lines
Op Sun, 14 Sep 2008, schreef Jonas Maebe: > > On 14 Sep 2008, at 14:49, Markus Beth wrote: > >> this patch rewrites code of ArrayRTTI and fpc_Copy to allow the >> compiler to generate faster code (at least on i386). >> The change in ArrayRTTI yields a performance gain of ~4% for our real >> world application on a 3 GHz Intel Xeon. >> The change in fpc_Copy is completely untestet because I don't know >> when this compilerproc is called. Maybe someone who knows can hack a >> quick benchmark... > > 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 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.

Shall I implement automatic simple loop induction variable tonight :)?

Reply by email

Daniël Mantione
to Florian Klaempfl

Op Sun, 14 Sep 2008, schreef Florian Klaempfl:

Show quoted text 28 lines
Daniël Mantione schrieb: > > > Op Sun, 14 Sep 2008, schreef Jonas Maebe: > >> >> On 14 Sep 2008, at 14:49, Markus Beth wrote: >> >>> this patch rewrites code of ArrayRTTI and fpc_Copy to allow the compiler >>> to generate faster code (at least on i386). >>> The change in ArrayRTTI yields a performance gain of ~4% for our real >>> world application on a 3 GHz Intel Xeon. >>> The change in fpc_Copy is completely untestet because I don't know when >>> this compilerproc is called. Maybe someone who knows can hack a quick >>> benchmark... >> >> 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 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. Shall I implement automatic simple loop induction variable tonight :)?

No, do it in a proper and maintainable way :) Seriously, we should not obstruct user contributions by raising questionable maintainability issues.

To the point: Any manual optimization can be done automatically, however, after 15 years of FPC we still are optimizing manually. That's the compiler writers full employment theorem.

Once the compiler can do these optimizations, then we can start to reject patches.

Daniël

Reply by email

Florian Klaempfl
to Daniël Mantione
Show quoted text 34 lines
Op Sun, 14 Sep 2008, schreef Florian Klaempfl: > Daniël Mantione schrieb: >> >> >> Op Sun, 14 Sep 2008, schreef Jonas Maebe: >> >>> >>> On 14 Sep 2008, at 14:49, Markus Beth wrote: >>> >>>> this patch rewrites code of ArrayRTTI and fpc_Copy to allow the >>>> compiler to generate faster code (at least on i386). >>>> The change in ArrayRTTI yields a performance gain of ~4% for our >>>> real world application on a 3 GHz Intel Xeon. >>>> The change in fpc_Copy is completely untestet because I don't know >>>> when this compilerproc is called. Maybe someone who knows can hack >>>> a quick benchmark... >>> >>> 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 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. > > Shall I implement automatic simple loop induction variable tonight :)? No, do it in a proper and maintainable way :)

I think this could be done already. DFA generates the necessary definition information. Expressions being candidates must be search anyways.

Reply by email

Jonas Maebe
to Daniël Mantione: Op Sun, 14 Sep 2008, schreef Jonas Maebe: This patch should get applied. It's debatable…
Daniël Mantione wrote 9 lines
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.

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.

Jonas

Reply by email

Daniël Mantione
to Jonas Maebe

Op Sun, 14 Sep 2008, schreef Jonas Maebe:

Show quoted text 16 lines
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.

Show quoted text 6 lines
> 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

Reply by email