Sign in Join

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

Mailing lists / fpc-devel / 2025-04

heap.inc evo when

15 messages · 6 people · –

Reply by email Subscribe to this list Your subscription

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

runewalsh

What’s the position on MR !694? Person with the yellow avatar mentioned in
https://gitlab.com/freepascal.org/fpc/source/-/commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all.

Life is short, you know. Especially if your de/allocations randomly become slow on their own (chopping up or finalizing the fixed OS chunk by the current heap.inc: extra 100 nanoseconds per allocation) and in addition to that hit
https://preshing.com/20110717/the-windows-heap-is-slow-when-launched-from-the-debugger/ (HeapAlloc under the debugger: extra 10 MICROseconds per allocation, absolutely terrible, I can watch e.g. dynamic strings being built and output one by one rather than instantly when hitting this case).

Reply by email

Florian Klämpfl
to runewalsh
runewalsh wrote
What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all.

Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review.

Show quoted text 13 lines
Life is short, you know. Especially if your de/allocations randomly become slow on their own (chopping up or finalizing the fixed OS chunk by the current heap.inc: extra 100 nanoseconds per allocation) and in addition to that hit https://preshing.com/20110717/the-windows-heap-is- slow-when-launched-from-the-debugger/ (HeapAlloc under the debugger: extra 10 MICROseconds per allocation, absolutely terrible, I can watch e.g. dynamic strings being built and output one by one rather than instantly when hitting this case). _______________________________________________ fpc-devel maillist  -  https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Reply by email

Michael Van Canneyt
to Florian Klämpfl
Florian Klämpfl wrote 9 lines
On 15.04.25 22:52, runewalsh via fpc-devel wrote: > What’s the position on MR !694? Person with the yellow avatar mentioned > in https://gitlab.com/freepascal.org/fpc/source/-/ > commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there > was some discussion somewhere, and that’s all. Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review.

Since Runewalsh is deep in it, clearly we're not off worse if we change it :-)

So, in order to progress: we'll throw it in the wild to be field-tested.

So I propose the following: I will add the new heap manager in a new file.

I'll include the old and the new one with an IFDEF, activated by default so it will be tested. In case of trouble the old one can be reactivated quickly using a simple recompile of the RTL.

If after 3(4,5,6) months we don't get bugreports, we remove the old one unless Runewalsh fixes the bugs.

Unless there are objections, I'll implement it this weekend.

Michael.

Reply by email

Florian Klämpfl
to Michael Van Canneyt
Show quoted text 16 lines
Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: > On 15.04.25 22:52, runewalsh via fpc-devel wrote: >> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. > > Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) So, in order to progress: we'll throw it in the wild to be field-tested. So I propose the following: I will add the new heap manager in a new file.

Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier.

Show quoted text 7 lines
I'll include the old and the new one with an IFDEF, activated by default so it will be tested. In case of trouble the old one can be reactivated quickly using a simple recompile of the RTL. If after 3(4,5,6) months we don't get bugreports, we remove the old one unless Runewalsh fixes the bugs. Unless there are objections, I'll implement it this weekend.

Fine with me.

Reply by email

Michael Van Canneyt
to Florian Klämpfl
Florian Klämpfl wrote 20 lines
> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : > > > > On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: > >> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >> >> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. > > Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) > > So, in order to progress: we'll throw it in the wild to be field-tested. > > So I propose the following: I will add the new heap manager in a new file. Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier.

I intended to do it more or less so, yes.

Michael.

Reply by email

Florian Klämpfl
to Michael Van Canneyt
Show quoted text 21 lines
Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel : On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > > >> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : >> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >>> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. >> Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) >> So, in order to progress: we'll throw it in the wild to be field-tested. >> So I propose the following: I will add the new heap manager in a new file. > > Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier. I intended to do it more or less so, yes.

The way you did it, line history is broken :(

Reply by email

Michael Van Canneyt
to Michael Van Canneyt: I intended to do it more or less so, yes. Michael.
Michael Van Canneyt wrote 35 lines
On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > > >> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel >> : >> >> >> >> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >> >>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>> What’s the position on MR !694? Person with the yellow avatar mentioned >>>> in https://gitlab.com/freepascal.org/fpc/source/-/ >>>> commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there >>>> was some discussion somewhere, and that’s all. >>> >>> Our main issue is that nobody really feels capable of reviewing it :( The >>> initial contributors of the heap manager are basically gone and nobody >>> else is really that deep into this code to do a review. >> >> Since Runewalsh is deep in it, clearly we're not off worse if we change it >> :-) >> >> So, in order to progress: we'll throw it in the wild to be field-tested. >> >> So I propose the following: I will add the new heap manager in a new file. > > Please do it in a way which preserves line history: copy heap.inc to > oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. > So heap.inc keeps line history making debugging in the future easier. I intended to do it more or less so, yes.

It's done.

-> renamed heap.inc to oldheap, change include in system.inc. -> added new heap.inc, used {$IFDEF LEGACYHEAP} to retain old heap manager.

So after every commit everything still compiles.

Michael.

Reply by email

Michael Van Canneyt
to Florian Klämpfl: The way you did it, line history is broken :(
Florian Klämpfl wrote 25 lines
> Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel : > > > > On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > >> >> >>> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : >>> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >>>> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. >>> Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) >>> So, in order to progress: we'll throw it in the wild to be field-tested. >>> So I propose the following: I will add the new heap manager in a new file. >> >> Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier. > > I intended to do it more or less so, yes. > The way you did it, line history is broken :(

Huh ? This makes no sense to me.

What's the point of having "git mv" if it does not preserve history?

Michael.

Reply by email

Michael Van Canneyt
to Michael Van Canneyt
Michael Van Canneyt wrote 45 lines
On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > > >> Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel >> : >> >> >> >> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >> >>> >>> >>>> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel >>>> : >>>> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>>>> What’s the position on MR !694? Person with the yellow avatar >>>>>> mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ >>>>>> commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there >>>>>> was some discussion somewhere, and that’s all. >>>>> Our main issue is that nobody really feels capable of reviewing it :( >>>>> The initial contributors of the heap manager are basically gone and >>>>> nobody else is really that deep into this code to do a review. >>>> Since Runewalsh is deep in it, clearly we're not off worse if we change >>>> it :-) >>>> So, in order to progress: we'll throw it in the wild to be field-tested. >>>> So I propose the following: I will add the new heap manager in a new >>>> file. >>> >>> Please do it in a way which preserves line history: copy heap.inc to >>> oldheap.inc, add and commit it. Then accept the merge request. Add >>> ifdefs. So heap.inc keeps line history making debugging in the future >>> easier. >> >> I intended to do it more or less so, yes. >> > > The way you did it, line history is broken :( Huh ? This makes no sense to me. What's the point of having "git mv" if it does not preserve history?

I checked, I can do "git log" on both files, and I get the full history (for oldheap.inc you need to add --follow to get it)

Is this not what you wanted ?

Michael.

Reply by email

Florian Klämpfl
to Michael Van Canneyt
Show quoted text 32 lines
Am 18.04.2025 um 15:10 schrieb Michael Van Canneyt via fpc-devel : On Fri, 18 Apr 2025, Michael Van Canneyt via fpc-devel wrote: > > > On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > >>> Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel : >>> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : >>>>> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>>>>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >>>>>> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. >>>>> Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) >>>>> So, in order to progress: we'll throw it in the wild to be field-tested. >>>>> So I propose the following: I will add the new heap manager in a new file. >>>> Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier. >>> I intended to do it more or less so, yes. >> The way you did it, line history is broken :( > > Huh ? This makes no sense to me. > > What's the point of having "git mv" if it does not preserve history? I checked, I can do "git log" on both files, and I get the full history (for oldheap.inc you need to add --follow to get it) Is this not what you wanted ?

When I do git blame I get:

c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 1) { c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 2) This file is part of the Free Pascal run time library. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 3) Copyright (c) 1999-2000 by the Free Pascal development team. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 4) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 5) functions for heap management in the data segment c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 6) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 7) See the file COPYING.FPC, included in this distribution, c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 8) for details about the copyright. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 9) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 10) This program is distributed in the hope that it will be useful, c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 11) but WITHOUT ANY WARRANTY; without even the implied warranty of c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 12) MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 13) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 14) **********************************************************************} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 15) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 16) {****************************************************************************} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 17) { Do not use standard memory manager } c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 18) { $define HAS_MEMORYMANAGER} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 19) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 20) { Memory manager } c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 21) {$if not defined(FPC_NO_DEFAULT_MEMORYMANAGER)} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 22) const c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 23) MemoryManager: TMemoryManager = ( c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 24) NeedLock: false; // Obsolete c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 25) GetMem: {$ifndef ; c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 26) FreeMem: {$ifndef

? Does it work for you?

Reply by email

Michael Van Canneyt
to Florian Klämpfl
Florian Klämpfl wrote 65 lines
> Am 18.04.2025 um 15:10 schrieb Michael Van Canneyt via fpc-devel : > > > > On Fri, 18 Apr 2025, Michael Van Canneyt via fpc-devel wrote: > >> >> >> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >> >>>> Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel : >>>> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>>> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : >>>>>> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>>>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>>>>>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >>>>>>> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. >>>>>> Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) >>>>>> So, in order to progress: we'll throw it in the wild to be field-tested. >>>>>> So I propose the following: I will add the new heap manager in a new file. >>>>> Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier. >>>> I intended to do it more or less so, yes. >>> The way you did it, line history is broken :( >> >> Huh ? This makes no sense to me. >> >> What's the point of having "git mv" if it does not preserve history? > > I checked, I can do "git log" on both files, and I get the full history (for > oldheap.inc you need to add --follow to get it) > > Is this not what you wanted ? When I do git blame I get: c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 1) { c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 2) This file is part of the Free Pascal run time library. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 3) Copyright (c) 1999-2000 by the Free Pascal development team. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 4) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 5) functions for heap management in the data segment c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 6) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 7) See the file COPYING.FPC, included in this distribution, c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 8) for details about the copyright. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 9) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 10) This program is distributed in the hope that it will be useful, c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 11) but WITHOUT ANY WARRANTY; without even the implied warranty of c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 12) MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 13) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 14) **********************************************************************} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 15) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 16) {****************************************************************************} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 17) { Do not use standard memory manager } c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 18) { $define HAS_MEMORYMANAGER} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 19) c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 20) { Memory manager } c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 21) {$if not defined(FPC_NO_DEFAULT_MEMORYMANAGER)} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 22) const c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 23) MemoryManager: TMemoryManager = ( c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 24) NeedLock: false; // Obsolete c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 25) GetMem: {$ifndef ; c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 26) FreeMem: {$ifndef ? Does it work for you?

No, I get the same.

But I only checked the workings of git log.

But now I understand correctly what you meant by "line history" : the "git blame" output. I implicitly assumed you meant 'linear history' :-(

My apologies, if I had correctly understood, I would have done exactly as you said :/

Michael.

Reply by email

Martin Frb
to Florian Klämpfl: When I do git blame I get: c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 …
Florian Klämpfl wrote 15 lines
> > I checked, I can do "git log" on both files, and I get the full > history (for > oldheap.inc you need to add --follow to get it) > > Is this not what you wanted ? When I do git blame I get: c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200   18) { $define HAS_MEMORYMANAGER} c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200   19)

Just an observation. This looks like
  git blame rtl/inc/heap.inc
the NEW file.

I think Michael means
  git blame rtl/inc/oldheap.inc
which will then show the former "heap.inc" too.

If you also want to be able to do the blame an the new heap.inc then you may have to squash the commits that remove it, and later add the new one.

But I am not sure if you then can do a good blame on oldheap.inc (would need to test that / not sure if duplication is seen as rename which will be followed).

Reply by email

Florian Klämpfl
to Michael Van Canneyt: No, I get the same. But I only checked the workings of git log. But now I understand…
Show quoted text 78 lines
Am 18.04.2025 um 15:44 schrieb Michael Van Canneyt via fpc-devel : On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: > > >> Am 18.04.2025 um 15:10 schrieb Michael Van Canneyt via fpc-devel : >> >> >> >> On Fri, 18 Apr 2025, Michael Van Canneyt via fpc-devel wrote: >> >>> >>> >>> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>> >>>>> Am 18.04.2025 um 14:07 schrieb Michael Van Canneyt via fpc-devel : >>>>> On Fri, 18 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>>>> Am 16.04.2025 um 12:00 schrieb Michael Van Canneyt via fpc-devel : >>>>>>> On Wed, 16 Apr 2025, Florian Klämpfl via fpc-devel wrote: >>>>>>>> On 15.04.25 22:52, runewalsh via fpc-devel wrote: >>>>>>>>> What’s the position on MR !694? Person with the yellow avatar mentioned in https://gitlab.com/freepascal.org/fpc/source/-/ commit/922457e5b58d76a13ff706f9220d9d6350631930#note_2390656703 there was some discussion somewhere, and that’s all. >>>>>>>> Our main issue is that nobody really feels capable of reviewing it :( The initial contributors of the heap manager are basically gone and nobody else is really that deep into this code to do a review. >>>>>>> Since Runewalsh is deep in it, clearly we're not off worse if we change it :-) >>>>>>> So, in order to progress: we'll throw it in the wild to be field-tested. >>>>>>> So I propose the following: I will add the new heap manager in a new file. >>>>>> Please do it in a way which preserves line history: copy heap.inc to oldheap.inc, add and commit it. Then accept the merge request. Add ifdefs. So heap.inc keeps line history making debugging in the future easier. >>>>> I intended to do it more or less so, yes. >>>> The way you did it, line history is broken :( >>> >>> Huh ? This makes no sense to me. >>> >>> What's the point of having "git mv" if it does not preserve history? >> >> I checked, I can do "git log" on both files, and I get the full history (for >> oldheap.inc you need to add --follow to get it) >> >> Is this not what you wanted ? > > When I do git blame I get: > > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 1) { > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 2) This file is part of the Free Pascal run time library. > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 3) Copyright (c) 1999-2000 by the Free Pascal development team. > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 4) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 5) functions for heap management in the data segment > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 6) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 7) See the file COPYING.FPC, included in this distribution, > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 8) for details about the copyright. > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 9) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 10) This program is distributed in the hope that it will be useful, > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 11) but WITHOUT ANY WARRANTY; without even the implied warranty of > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 12) MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 13) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 14) **********************************************************************} > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 15) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 16) {****************************************************************************} > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 17) { Do not use standard memory manager } > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 18) { $define HAS_MEMORYMANAGER} > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 19) > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 20) { Memory manager } > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 21) {$if not defined(FPC_NO_DEFAULT_MEMORYMANAGER)} > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 22) const > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 23) MemoryManager: TMemoryManager = ( > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 24) NeedLock: false; // Obsolete > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 25) GetMem: {$ifndef ; > c8bf474d6d0 (Michaël Van Canneyt 2025-04-18 14:44:24 +0200 26) FreeMem: {$ifndef > > ? Does it work for you? No, I get the same. But I only checked the workings of git log. But now I understand correctly what you meant by "line history" : the "git blame" output. I implicitly assumed you meant 'linear history' :-(

Yes :) It often helps to understand code. But I think I can fix it without rewriting git history. Though that one of oldheap.inc will be gone but it doesn’t matter for oldheap.inc imo.

Show quoted text 6 lines
My apologies, if I had correctly understood, I would have done exactly as you said :/ Michael._______________________________________________ fpc-devel maillist - https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Reply by email

Maxim Ganetsky
to Florian Klämpfl

18.04.2025 16:53, Florian Klämpfl via fpc-devel пишет:

Show quoted text 13 lines
>> ? Does it work for you? > > No, I get the same. > > But I only checked the workings of git log. > > But now I understand correctly what you meant by "line history" : the > "git blame" > output. I implicitly assumed you meant 'linear history' :-( Yes :) It often helps to understand code. But I think I can fix it without rewriting git history. Though that one of oldheap.inc will be gone but it doesn’t matter for oldheap.inc imo.

In related merge request I see that in heap.inc 1400 lines of 1700 were touched anyway, so git blame will be of limited use anyway?

Best regards, Maxim

Reply by email

Pierre Muller
to Florian Klämpfl: Yes :) It often helps to understand code. But I think I can fix it without rewriting git…

Florian, you committed several changes to fix history, but they somehow resulted in the fact that oldheap.inc is not present in current trunk...

This means that it is impossible to test old heap, could you please reinstall oldheap or explain
what happens.

Pierre

make rtl OPT="-n -vx -dLEGACYHEAP"
make -C rtl all
make[1]: Entering directory '/home/muller/pas/gitlab-fpc-source/rtl' make -C linux all
make[2]: Entering directory '/home/muller/pas/gitlab-fpc-source/rtl/linux' /usr/bin/mkdir -p /home/muller/pas/gitlab-fpc-source/rtl/units/x86_64-linux make /home/muller/pas/gitlab-fpc-source/rtl/units/x86_64-linux make[3]: Entering directory '/home/muller/pas/gitlab-fpc-source/rtl/linux' make[3]: '/home/muller/pas/gitlab-fpc-source/rtl/units/x86_64-linux' is up to date. make[3]: Leaving directory '/home/muller/pas/gitlab-fpc-source/rtl/linux' as --64 -o /home/muller/pas/gitlab-fpc-source/rtl/units/x86_64-linux/abitag.o x86_64/abitag.as /home/muller/pas/fpc-3.3.1/bin/ppcx64 -Ur -Ur -Xs -O2 -n -Fi../inc -Fi../x86_64 -Fi../unix -Fix86_64 -FE. -FU/home/muller/pas/gitlab-fpc-source/rtl/units/x86_64-linux -Cg -Fl/usr/lib/gcc/x86_64-linux-gnu/12 -n -vx -dLEGACYHEAP -dx86_64 -dRELEASE -Us -Sg system.pp system.inc(1780,2) Fatal: Cannot open include file "oldheap.inc" Fatal: Compilation aborted

grep oldheap -iwnrC4 .
./system.inc-1776-
./system.inc-1777-{$i sysheap.inc}
./system.inc-1778-
./system.inc-1779-{$IFDEF LEGACYHEAP}
./system.inc:1780:{$i oldheap.inc}
./system.inc-1781-{$ELSE}
./system.inc-1782-{$i heap.inc}
./system.inc-1783-{$ENDIF LEGACYHEAP}
./system.inc-1784-

git log -n 6 -- rtl/inc/oldheap.inc
commit dfbd4eac2a81a88c0e11367d0159781c2e254821
Author: florian
Date: Fri Apr 18 14:55:13 2025 +0200

+ fix line history part 2

commit 06aff099c163330f64f6a871ebe4ede1fabe7286
Author: Michaël Van Canneyt
Date: Fri Apr 18 14:18:41 2025 +0200

* Move heap to oldheap, prepare for MR !694

Reply by email