Re: Proposal for Prototype Implementation to Enhance C/C++ Interoperability in PostgreSQL

2023-05-09 Thread
I apologize for my previous hasty conclusion. I have conducted further testing 
on different platforms and would like to share my findings.

> FreeBSD

Based on my tests, it appears that FreeBSD follows the Itanium C++ ABI 
specification. The previous test failed because the C++ compiler was not used 
when linking libpgsjlj.so.

> macOS, M1

My tests show that macOS M1 roughly follows the Itanium C++ ABI specification, 
with only slight differences, such as the parameters accepted by the 
_Unwind_Stop_Fn function.

> macOS, x86

I don't have the resources to do the testing, but from a code perspective, it 
appears that macOS x86 follows the Itanium C++ ABI specification.

> Windows

It seems that Windows does not follow the Itanium C++ ABI specification at all. 
If we compile the program using the `/EHsc` option, longjmp will also trigger 
forced unwinding. However, unlike the Itanium C++ ABI, the forced unwinding 
triggered here cannot be captured by a C++ catch statement.

Re: Proposal for Prototype Implementation to Enhance C/C++ Interoperability in PostgreSQL

2023-05-07 Thread
(Sorry, there was a problem with the format of the previous email content. I 
will send it in plain text format this time

> It seems extremely specific to one particular C++ implementation

To perform a force unwind during longjmp, the _Unwind_ForcedUnwind function is 
used. This function is defined in the [Itanium C++ ABI 
Standard](https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw), 
which is followed by all C++ implementations. Additionally, the glibc 
[nptl/unwind.c](https://elixir.bootlin.com/glibc/latest/source/nptl/unwind.c#L130)
 file shows that on all platforms, pthread_exit is also implemented using 
_Unwind_ForcedUnwind.

Furthermore, the Itanium C++ ABI specification also defines 
_Unwind_RaiseException as the entry point for all C++ exceptions thrown.

> you've thrown in a new dependency on pthreads

The reason for the dependence on pthread is due to the overloading of 
_Unwind_RaiseException, which serves as the entry point for all C++ throwing 
exceptions. Some third-party C++ libraries may create threads internally and 
throw exceptions.

Overloading _Unwind_RaiseException is done to convert uncaught exceptions into 
elog(ERROR). If we require that all exceptions must be caught, we can remove 
the overloading of _Unwind_RaiseException and all pthread dependencies.

The overloading of _Unwind_RaiseException is just a fallback measure to prevent 
uncaught exceptions from terminating the process. In our code, this path is 
rarely taken, and once we encounter an exception that is not caught, we will 
fix the code to catch the exception.

> doesn't this require us to move our minimum language requirement to 
> C++-something?

No, all code has no dependency on C++.

regards, 盏一

Re: Proposal for Prototype Implementation to Enhance C/C++ Interoperability in PostgreSQL

2023-05-07 Thread
 It seems extremely specific to one particular C++ implementation


To perform a force unwind during longjmp, the _Unwind_ForcedUnwind function is 
used. This function is defined in the [Itanium C++ ABI 
Standard](https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw), 
which is followed by all C++ implementations. Additionally, the glibc 
[nptl/unwind.c](https://elixir.bootlin.com/glibc/latest/source/nptl/unwind.c#L130)
 file shows that on all platforms, pthread_exit is also implemented using 
_Unwind_ForcedUnwind.


Furthermore, the Itanium C++ ABI specification also defines 
_Unwind_RaiseException as the entry point for all C++ exceptions thrown.


 you've thrown in a new dependency on pthreads



The reason for the dependence on pthread is due to the overloading of 
_Unwind_RaiseException, which serves as the entry point for all C++ throwing 
exceptions. Some third-party C++ libraries may create threads internally and 
throw exceptions.


Overloading _Unwind_RaiseException is done to convert uncaught exceptions into 
elog(ERROR). If we require that all exceptions must be caught, we can remove 
the overloading of _Unwind_RaiseException and all pthread dependencies.


The overloading of _Unwind_RaiseException is just a fallback measure to prevent 
uncaught exceptions from terminating the process. In our code, this path is 
rarely taken, and once we encounter an exception that is not caught, we will 
fix the code to catch the exception.


 doesn't this require us to move our minimum language requirement to 
C++-something?



No, all code has no dependency on C++.

--Original--
From: "t...@sss.pgh.pa.us"https://github.com/postgres/postgres/commit/1a9a2790430f256d9d0cc371249e43769d93eb8e#diff-6b6034caa00ddf38f641cbd10d5a5d1bb7135f8b23c5a879e9703bd11bd8240f).
 I would appreciate it if you could review the implementation and provide 
feedback.

... but I think this patch has no hope of being adequately portable.
It seems extremely specific to one particular C++ implementation
(unless you can show that every single thing you've used here is
in the C++ standard), and then for good measure you've thrown in
a new dependency on pthreads. On top of that, doesn't this
require us to move our minimum language requirement to C++-something?
We just barely got done deciding C99 was okay to use.

 regards, tom lane

Proposal for Prototype Implementation to Enhance C/C++ Interoperability in PostgreSQL

2023-05-07 Thread
Hi




I am writing to propose a prototype implementation that can greatly enhance the 
C/C++ interoperability in PostgreSQL. The implementation involves converting PG 
longjmp to[force 
unwind](https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw), 
which triggers the destruction of local variables on the stack. Additionally, 
it converts throw statements that are not associated with catch to PG longjmp, 
thereby avoiding the call to terminate.




The proposed implementation can significantly improve the interoperability 
between C and C++ code in PostgreSQL. It allows for seamless integration of C++ 
code with PostgreSQL, without the need for complex workarounds or modifications 
to the existing codebase.




I have submitted the implementation 
on[GitHub](https://github.com/postgres/postgres/commit/1a9a2790430f256d9d0cc371249e43769d93eb8e#diff-6b6034caa00ddf38f641cbd10d5a5d1bb7135f8b23c5a879e9703bd11bd8240f).
 I would appreciate it if you could review the implementation and provide 
feedback.




Thank you for your time and consideration.




Best regards,

盏一
--

Re: use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`

2021-06-11 Thread
Ok! Thanks
--




--原始邮件--
发件人:"Andres Freund "

Re: use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`

2021-05-06 Thread
Sounds like a plan! Do you want to write a patch?
Add the patch.

0001-use-pgxactoff-as-the-value-of-index-in-ProcArrayRemo.patch
Description: Binary data


use `proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`

2021-05-06 Thread
Hi,

Since we have introduced `pgxactoff` in 
[941697c3c1ae5d6ee153065adb96e1e63ee11224](https://github.com/postgres/postgres/commit/941697c3c1ae5d6ee153065adb96e1e63ee11224),
 and `pgxactoff` is always the index of `proc->pgprocno` in 
`procArray->pgprocnos`. So it seems that we could directly use 
`proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`? My thought is 
to replace 

```c
for (index = 0; index < arrayP->numProcs; index++)
{
  if (arrayP->pgprocnos[index] == proc->pgprocno)
  {
  /* ... */
  }
}
```

with 

```c
index = proc->pgxactoff;
/* ... */
```

I would appreciate your help.

Re:Issue about memory order on ARM

2019-12-01 Thread
Sorry to bother you, now I know that there is no problem here.

The model for reading and writing of PGXACT::xid and 
ShmemVariableCache->latestCompletedXid can be simplified as follows:

 backend A backend B   
backend C
  wlock(XidGenLock);   wlock(XidGenLock);   
rlock(ProcArrayLock);
  write APgXact->xid; write BPgXact->xid;   read 
latestCompletedXid;
  unlock(XidGenLock); unlock(XidGenLock);  read 
APgXact->xid;
  ...   
read BPgXact->xid;
  wlock(ProcArrayLock);
unlock(ProcArrayLock);
  write latestCompletedXid;
  unlock(ProcArrayLock);

My previous problem was that C might not be able to see the value of 
APgXact->xid written by A because there was no obvious acquire-release 
operation during this. But now I find that there are already some 
acquire-release operations here. Because of the `unlock(XidGenLock)` in A and 
`wlock(XidGenLock)` in B and the rules introduced in [Inter-thread 
happens-before](https://en.cppreference.com/w/cpp/atomic/memory_order), we can 
know that the `write APgXact->xid` in A inter-thread happens before `write 
BPgXact->xid` in B. And `write BPgXact->xid` is sequenced before `write 
latestCompletedXid` in B according to rules introduced in [Sequenced-before 
rules](https://en.cppreference.com/w/cpp/language/eval_order). And similarly 
`write latestCompletedXid` in B inter-thread happens before `read 
latestCompletedXid` in C. So the `write APgXact->xid` in A inter-thread happens 
before `read APgXact->xid` in C. So C can see the value of APgXact->xid written 
by A.

Issue about memory order on ARM

2019-11-30 Thread
The code in GetSnapshotData() that read the `xid` field of PGXACT struct 
has a dependency on code in GetNewTransactionId() that write 
`MyPgXact-xid`. It means that the store of xid should happen before the 
load of it. In C11, we can use [Release-Acquire 
ordering](https://en.cppreference.com/w/c/atomic/memory_order#Release-Acquire_ordering)
 to achieve it. But now, there is no special operation to do it(, and the 
[volatile](https://en.cppreference.com/w/c/language/volatile) keyword should 
not play any role in this situation).


So it means that when a backend A returns from GetNewTransactionId(), the 
newval of `MyPgXact-xid` maybe just in CPU store buffer, or CPU cache line, 
so the newval is not yet visible to backend B that calling GetSnapshotData(). 
So the assumption that 'all top-level XIDs <= latestCompletedXid are either 
present in the ProcArray, or not running anymore' may not be safe.