Boost

[Adriano dos Santos Fernandes]

Hi!

We currently have a subset (talking about extern/boost, not
src/include/firebird/impl/boost) of boost library integrated in our
code. It's used only for unit tests.

I propose we start to have open mind to discuss more adoption of it,
considering case-by-case, portability and performance. Specially its
header-only libraries, which integrates easily in our build.

A good example is IPC, which we have lot of OS-dependent, inflexible and
difficult to use code. And as pointed today, we have not even a simple
shared mutex.

The profiler has a bug
(https://github.com/FirebirdSQL/firebird/issues/8639) and I see it can
be easily fixed using boost::interprocess::interprocess_mutex.

Boost is a success case and inspired a lot modern C++ std features. We'd
better use it than send time creating and maintaining complex non-core code.


Adriano

[Dimitry Sibiryakov]

Adriano dos Santos Fernandes wrote 09.07.2025 4:09:
> A good example is IPC, which we have lot of OS-dependent, inflexible and
> difficult to use code. And as pointed today, we have not even a simple
> shared mutex.

Windows mutexes are shared out of box. Linux mutexes cannot work without
shared memory so BaseSharedMemory is what you need anyway.

--
WBR, SD.

[Dimitry Sibiryakov]

Adriano dos Santos Fernandes wrote 09.07.2025 4:09:

> The profiler has a bug
> (https://github.com/FirebirdSQL/firebird/issues/8639) and I see it can
> be easily fixed using boost::interprocess::interprocess_mutex.

Why can't profiler reuse code from trace which is stable enough?

--
WBR, SD.

[Vlad Khorsun]

09.07.2025 5:09, Adriano dos Santos Fernandes:

> Hi!
>
> We currently have a subset (talking about extern/boost, not
> src/include/firebird/impl/boost) of boost library integrated in our
> code. It's used only for unit tests.
>
> I propose we start to have open mind to discuss more adoption of it,
> considering case-by-case, portability and performance. Specially its
> header-only libraries, which integrates easily in our build.

No objection in general.

> A good example is IPC, which we have lot of OS-dependent, inflexible and
> difficult to use code. And as pointed today, we have not even a simple
> shared mutex.

Here I'm not agree. We know our code very well and it is not difficult to use.
What is very important - we can fix and adapt it when needed. And we have

shared mutex.
> The profiler has a bug
> (https://github.com/FirebirdSQL/firebird/issues/8639) and I see it can
> be easily fixed using boost::interprocess::interprocess_mutex.

Could you explain ? What boost's mutex have that we haven't ? Maybe we can
help you ?

> Boost is a success case and inspired a lot modern C++ std features. We'd
> better use it than send time creating and maintaining complex non-core code.

First, I have no problem adapting external libraries, especially well-known
and well maintained, as boost. But only when really necessary, for example if
it implements something we missing. Next, IPC is an essential part of Firebird
core and I see no reason to change its implementation just because someone not
like it or not understand it. There must be more strong arguments.

Regards,
Vlad

[Adriano dos Santos Fernandes]

On 09/07/2025 06:30, Vlad Khorsun wrote:
> 09.07.2025 5:09, Adriano dos Santos Fernandes:
>> Hi!
>>
>> We currently have a subset (talking about extern/boost, not
>> src/include/firebird/impl/boost) of boost library integrated in our
>> code. It's used only for unit tests.
>>
>> I propose we start to have open mind to discuss more adoption of it,
>> considering case-by-case, portability and performance. Specially its
>> header-only libraries, which integrates easily in our build.
>
>   No objection in general.

Great.

>> A good example is IPC, which we have lot of OS-dependent, inflexible and
>> difficult to use code. And as pointed today, we have not even a simple
>> shared mutex.
>
>   Here I'm not agree. We know our code very well and it is not difficult
> to use.

You helped a lot to make profiler IPC work after my initial bugged
implementation.

And it still has a hidden bug that I found myself difficult to fix
without an extra shared mutex. Of course, any implementation could have
bugs, but in this case I think I still have a point.

> What is very important - we can fix and adapt it when needed. And we have
> shared mutex.
>> The profiler has a bug
>> (https://github.com/FirebirdSQL/firebird/issues/8639) and I see it can
>> be easily fixed using boost::interprocess::interprocess_mutex.
>   Could you explain ? What boost's mutex have that we haven't ? Maybe we
> can
> help you ?

Sure, but I don't want to discuss specifics of this bug in this topic.

Actually, I detected this bug exploring a different new feature that I'm
still going to propose. So mixing three topics here is not going to help.

>> Boost is a success case and inspired a lot modern C++ std features. We'd
>> better use it than send time creating and maintaining complex non-core
>> code.
>   First, I have no problem adapting external libraries, especially well-
> known
> and well maintained, as boost. But only when really necessary, for
> example if
> it implements something we missing. Next, IPC is an essential part of
> Firebird
> core and I see no reason to change its implementation just because
> someone not
> like it or not understand it. There must be more strong arguments.
>

I used it many times and all times it was difficult and probably there
is still bugs left.

There is no good documentation and of course, no widely usage (it's
exclusive in Firebird).

But I'm not proposing to start replacing Firebird IPC by boost code.

This is a proposal of boost as a general enabler, which an example of
it's IPC library.

I also talked about performance, so if third party code is easy to use
but do not meet current Firebird performance expectations were it really
matter, that third party code should not be used.


Adriano

[Adriano dos Santos Fernandes]

I created https://github.com/FirebirdSQL/firebird/pull/8645 to
demonstrate fix using boost IPC mutex and how I think it provides
something easily usable that we do not yet have.


Adriano

[Jim Starkey]

Did Firebird ever import SyncObject from Vulcan?  If not, they are user
mode read/write locks with Java monitor-like semantics so threads don't
lock against themselves.  Very, very fast, simple, and don't involve
inheriting a billion lines of code.  Used with a stack local Sync
object, they can be automatically released when a block exits.

For databases, I hate using massively bloated external libraries.  For
other stuff, sure.  Having objects explicitly designed for a heavily
controlled environment is usually a huge win.

--
Jim Starkey

[Vlad Khorsun]

09.07.2025 17:33, Jim Starkey:

> Did Firebird ever import SyncObject from Vulcan?  If not, they are user mode read/write locks with Java monitor-like semantics so
> threads don't lock against themselves.  Very, very fast, simple, and don't involve inheriting a billion lines of code.  Used with a
> stack local Sync object, they can be automatically released when a block exits.

SyncObject used heavily since Firebird3. But it can't be put into shared memory as is -
it is contains pointers to the wait queue (contains ThreadSync's).

> For databases, I hate using massively bloated external libraries.  For other stuff, sure.  Having objects explicitly designed for a
> heavily controlled environment is usually a huge win.

Fully agree.

Regards,
Vlad

[James Starkey]

Got it.  Let me ponder that.  I never, ever use share memory (at least since Interbase), strongly preferring “shared nothing” architectures.  Pox on lock managers everywhere.

Jim Starkey

--
Support the ongoing development of Firebird! Consider donating to the Firebird Foundation and help ensure its future. Every contribution makes a difference. Learn more and donate here:
https://www.firebirdsql.org/donate
---
You received this message because you are subscribed to the Google Groups "firebird-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firebird-deve...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/firebird-devel/2a09bc30-f917-4ea3-9cee-534907b46557%40gmail.com.

[Vlad Khorsun]

09.07.2025 17:16, Adriano dos Santos Fernandes:

> I created https://github.com/FirebirdSQL/firebird/pull/8645 to
> demonstrate fix using boost IPC mutex and how I think it provides
> something easily usable that we do not yet have.

I didn't build and run it so far. Could you explain if boost mutex creates any
new files for own needs ? Also, what memory manager is used by std::vector ?

Regards,
Vlad

PS In general, there is no need to have two mutexes for this task, single
sh_mem_mutex is enough.

[Vlad Khorsun]

10.07.2025 0:25, Vlad Khorsun:

> 09.07.2025 17:16, Adriano dos Santos Fernandes:
>> I created https://github.com/FirebirdSQL/firebird/pull/8645 to
>> demonstrate fix using boost IPC mutex and how I think it provides
>> something easily usable that we do not yet have.
>
>   I didn't build and run it so far. Could you explain if boost mutex creates any
> new files for own needs ?

AFAIU, on Windows it creates named mutex with name prefixed by "Global\\boost.ipc".
I.e. it not uses Windows private namespaces and return us the issues with communication
between non-service and service processes.

Regards,
Vlad

[Adriano dos Santos Fernandes]

I was going to reply and saw you have analyzed it.

Yes, but not by default. By default it uses a spin lock. I think it's not a good default.

And with a define it can use that Global\\boost.ipc or pthread shared mutex.

So good, we have to analyze docs and/or implementation.

It gave me an idea that the profiler IPC buffer may be protected with a spin lock, as it's a very short operation that will rarely have a race.

Adriano

[Adriano dos Santos Fernandes]

On Wed, Jul 9, 2025 at 6:25 PM Vlad Khorsun <fbv...@gmail.com> wrote:

Also, what memory manager is used by std::vector ?

It's a stack variable, so I would not worry about it.

But a HalfStaticArray would be more appropriate there as usually the used space will be very low. The buffer actually is oversized.

 

PS In general, there is no need to have two mutexes for this task, single
sh_mem_mutex is enough.

If that was true, there was not that bug.

Adriano

 

[Dimitry Sibiryakov]

Adriano dos Santos Fernandes wrote 10.07.2025 4:27:
>> PS In general, there is no need to have two mutexes for this task, single
>> sh_mem_mutex is enough.
>
> If that was true, there was not that bug.

If that was false, "that bug" would be in trace as well, right?

--
WBR, SD.

[Vlad Khorsun]

10.07.2025 5:27, Adriano dos Santos Fernandes:

> On Wed, Jul 9, 2025 at 6:25 PM Vlad Khorsun <fbv...@gmail.com <mailto:fbv...@gmail.com>> wrote:

> PS In general, there is no need to have two mutexes for this task, single
> sh_mem_mutex is enough.
>
>
> If that was true, there was not that bug.

The bug is because of misuse of sh_mem_mutex.

If you wish, I'll explain more or prepare a patch with what I consider as correct
usage of sh_mem_mutex and not requiring additional mutexes.

Regards,
Vlad

[Adriano dos Santos Fernandes]

Yes.


Adriano

[Vlad Khorsun]

11.07.2025 3:16, Adriano dos Santos Fernandes:

Branch work/gh-8639/profiler-ipc-2

Regards,
Vlad

[Adriano dos Santos Fernandes]

I compared both solutions and for me, your solution is way more complex
and less performant in the common scenario: when there is no query being
cancelled. With it, many clients and the server competes for a lock
which would better be a conditional variable.

In my solution, there is the disadvantage of the spinning lock, but it
"spins" only in the rare event of a query being cancelled.


Adriano

[Vlad Khorsun]

14.07.2025 14:17, Adriano dos Santos Fernandes:

> On 13/07/2025 19:19, Vlad Khorsun wrote:
>> 11.07.2025 3:16, Adriano dos Santos Fernandes:
>>> On 10/07/2025 17:11, Vlad Khorsun wrote:
>>>> 10.07.2025 5:27, Adriano dos Santos Fernandes:
>>>>> On Wed, Jul 9, 2025 at 6:25 PM Vlad Khorsun <fbv...@gmail.com
>>>>> <mailto:fbv...@gmail.com>> wrote:
>>>>
>>>>>      PS In general, there is no need to have two mutexes for this task,
>>>>> single
>>>>>      sh_mem_mutex is enough.
>>>>>
>>>>>
>>>>> If that was true, there was not that bug.
>>>>    The bug is because of misuse of sh_mem_mutex.
>>>>
>>>>    If you wish, I'll explain more or prepare a patch with what I
>>>> consider
>>>> as correct
>>>> usage of sh_mem_mutex and not requiring additional mutexes.
>>>>
>>>
>>> Yes.
>>
>>   Branch work/gh-8639/profiler-ipc-2
>>
>
> I compared both solutions and for me, your solution is way more complex
> and less performant in the common scenario: when there is no query being
> cancelled. With it, many clients and the server competes for a lock
> which would better be a conditional variable.

I don't want to argue, while I not agree with all points above.

> In my solution, there is the disadvantage of the spinning lock, but it
> "spins" only in the rare event of a query being cancelled.

This is not the only problem of the spinlock.

Regards,
Vlad

PS The main goal was to show that sh_mem_mutex should be used to protect
shared memory from concurrent access, not to just serialize clients, allowing
server wild asynchronous access to the shared memory, as it was in first
implementation.

[Alex Peshkoff]

On 7/10/25 05:27, Adriano dos Santos Fernandes wrote:
>
> PS In general, there is no need to have two mutexes for this task,
> single
> sh_mem_mutex is enough.
>
>

Sorry for being late here - want to add my 2 cents.

In theory one single mutex is enough for any task - remember SS before
fb2.5. But it's also obvious that such solution is typically not optimal
for multi core system. What about single mutex for shared memory file -
it was done because we never had a need ti have additional mutex in it.
Bit if profiler requires shared mutex - why not move it from shared
memory file into separate class which may be used in shared memory file?
I suppose this can be even done w/o changes in shared memory layout but
fro FB6 even need of such change is not a problem.

PS. In general I also agree that we need an open look at adding various
standard libraries to our code - did it myself sometimes:) - but suppose
use of boost here is not that case.