Your simplified code would not be thread safe. A thread swap after the
first instruction of
mov (%rdi), %rax
could potentially cause a problem. RAX (A) contains the pointer to the
string descriptor, which includes the pointer to the actual string
data and the reference count.
Should "A" have a reference count of 1, and it is assigned a new value
on another thread, its reference count will decrease to zero and the
string and its descriptor are both freed. The value of "A" will be set
to nil.
RAX will not have been updated, so after
mov %rax, (%rsi)
"B" will be pointing to an invalid location. Its content is now
dependent on memory reallocation.
In this situation, changing "B" after the return from stringSwap
procedure might again cause the string to be freed (assuming the
memory was not reallocated), causing a double free error.
Use of the internal procedures to increment the string reference count
of both strings before your code and then decrementing the reference
counts afterwards would probably work.
The same probably applies to other reference count objects.
Derek
On Thu, Jan 28, 2021 at 11:35 AM Benito van der Zander via fpc-pascal
<fpc-pascal@lists.freepascal.org
<mailto:fpc-pascal@lists.freepascal.org>> wrote:
Hi,
Procedure ManagedMove<T>(const source: T;var dest: T;count:
SizeInt);
In principle a good idea. However this is one of those cases
where you'd definitely need to use constref instead of const.
Or var, since the source might be cleared
And perhaps there could be a special attribute to mark which
kind of moving is needed, e.g..
type [moveable] TA = record
type [referencecounted] TA = record
type [nonmoveable] TA = record
No, thank you.
But it could help a lot with optimizations. For moveable types it
could omit calling an assignment function and just do a memory
copy; and for refcounted tyes it could that when the number of
references does not change.
Like a simple swap function:
procedure stringSwap(var a, b: string);
var t: string;
begin
t := a;
a := b;
b := t;
end;
A smart compiler could detect that it is a refcounted type and the
number of references stays the same, and thus optimize it into:
mov (%rdi),%rax
mov (%rsi),%rdx
mov %rdx,(%rdi)
mov %rax,(%rsi)
retq
But FPC turns it into:
begin
push %rbx
push %r12
lea -0x68(%rsp),%rsp
mov %rdi,%rbx
mov %rsi,%r12
movq $0x0,(%rsp)
lea 0x8(%rsp),%rdx
lea 0x20(%rsp),%rsi
mov $0x1,%edi
callq 0x4324c0 <fpc_pushexceptaddr>
mov %rax,%rdi
callq 0x41fba0 <fpc_setjmp>
movslq %eax,%rdx
mov %rdx,0x60(%rsp)
test %eax,%eax
jne 0x469191 <STRINGSWAP+97>
t := a;
mov (%rbx),%rsi
mov %rsp,%rdi
callq 0x428d00 <fpc_ansistr_assign>
a := b;
mov (%r12),%rsi
mov %rbx,%rdi
callq 0x428d00 <fpc_ansistr_assign>
b := t;
mov (%rsp),%rsi
mov %r12,%rdi
callq 0x428d00 <fpc_ansistr_assign>
callq 0x432830 <fpc_popaddrstack>
end;
mov %rsp,%rdi
callq 0x428ca0 <fpc_ansistr_decr_ref>
mov 0x60(%rsp),%rax
test %rax,%rax
je 0x4691b8 <STRINGSWAP+136>
callq 0x4329e0 <fpc_reraise>
movq $0x0,0x60(%rsp)
jmp 0x469191 <STRINGSWAP+97>
lea 0x68(%rsp),%rsp
pop %r12
pop %rbx
retq
Then you want to simply use ismanagedtype and move(). Moving the
move() to a separate generic procedure will only lead to many
instantiations of what is basically a move() procedure.
The goal is to reduce instantiations.
If there are n generic (collection) classes, C1, C2, .., Cn, and k
types used in the collections, T1, T2, .. Tk, there are n*k
collection class instantiation C1<T1>, C1<T2>, .. C1<Tk>, C2<T1>,
... Cn<Tk>
If each collection class does its own Finalize-Loop/Move/Fillchar
there are also n*k of these move implementations. So like 3*n*k calls.
When any collection can call the same ManagedMove, there should
only be k ManagedMove instantiations and n*k calls to ManagedMove,
which is only one call. So it is k + n*k calls, which is around a
third of 3*n*k
Bye,
Benito
On 11.01.21 18:51, Sven Barth via fpc-pascal wrote:
Benito van der Zander via fpc-pascal
<fpc-pascal@lists.freepascal.org
<mailto:fpc-pascal@lists.freepascal.org>> schrieb am Mo., 11.
Jan. 2021, 15:26:
Hi,
perhaps a safe, generic function for this copying could be
added to the RTL. Like:
Procedure ManagedMove<T>(const source: T;var dest: T;count:
SizeInt);
In principle a good idea. However this is one of those cases
where you'd definitely need to use constref instead of const.
And when you use IsManagedType, it does not distinguish
standard strings with such weird managed types.
You can additionally use GetTypeKind as well. Unlike TypeInfo it
directly returns the TTypeKind (which for this case is enough)
and is considered constant.
And perhaps there could be a special attribute to mark which
kind of moving is needed, e.g..
type [moveable] TA = record
type [referencecounted] TA = record
type [nonmoveable] TA = record
No, thank you.
Regards,
Sven
_______________________________________________
fpc-pascal maillist -fpc-pascal@lists.freepascal.org
<mailto:fpc-pascal@lists.freepascal.org>
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
<https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal>
_______________________________________________
fpc-pascal maillist - fpc-pascal@lists.freepascal.org
<mailto:fpc-pascal@lists.freepascal.org>
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
<https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal>
--
Derek Edson
_______________________________________________
fpc-pascal maillist - fpc-pascal@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
