Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Boehm-Demers-Weiser Garbage Collector. #647

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Conversation

dimitri
Copy link
Owner

@dimitri dimitri commented Jan 22, 2024

From their own documentation:

The Boehm-Demers-Weiser conservative garbage collector can be used as a
garbage collecting replacement for C malloc or C++ new. It allows you to
allocate memory basically as you normally would, without explicitly
deallocating memory that is no longer useful. The collector automatically
recycles memory when it determines that it can no longer be otherwise
accessed.

In pgcopydb C code base it's been hard to maintain proper malloc/free concerns while also implementing proper error-handling, which is a common problem when writing C code. In order to ease the maintenance of the code and reduce production hazards, the best way to proceed looks like automating the memory management altogether by using a Garbage Collector.

Replaces and close #613
Should fix #609

@dimitri dimitri added bug Something isn't working enhancement New feature or request labels Jan 22, 2024
@dimitri dimitri added this to the v0.16 milestone Jan 22, 2024
@dimitri dimitri self-assigned this Jan 22, 2024
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
src/bin/pgcopydb/defaults.h Show resolved Hide resolved
src/bin/pgcopydb/pgcmd.c Show resolved Hide resolved
src/bin/lib/subcommands.c/runprogram.h Show resolved Hide resolved
src/bin/lib/subcommands.c/runprogram.h Show resolved Hide resolved
From their own documentation:

> The Boehm-Demers-Weiser conservative garbage collector can be used as a
> garbage collecting replacement for C malloc or C++ new. It allows you to
> allocate memory basically as you normally would, without explicitly
> deallocating memory that is no longer useful. The collector automatically
> recycles memory when it determines that it can no longer be otherwise
> accessed.

In pgcopydb C code base it's been hard to maintain proper malloc/free
concerns while also implementing proper error-handling, which is a common
problem when writing C code. In order to ease the maintenance of the code
and reduce production hazards, the best way to proceed looks like automating
the memory management altogether by using a Garbage Collector.
Now that libgc is handling the memory for us, we don't need to keep track of
the ownership of the buffer anymore.
Copy link
Contributor

@shubhamdhama shubhamdhama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested locally with "lots" of database objects and didn't observed any anomaly in system memory after considerable period of time (not the best way to test this change but the most convenient I had).

@dimitri dimitri merged commit 29a4268 into main Jan 25, 2024
18 checks passed
@dimitri dimitri deleted the fix/use-garbase-collector branch January 25, 2024 17:13
gokhangulbiz pushed a commit to gokhangulbiz/pgcopydb that referenced this pull request Jan 26, 2024
* Use Boehm-Demers-Weiser Garbage Collector.

From their own documentation:

> The Boehm-Demers-Weiser conservative garbage collector can be used as a
> garbage collecting replacement for C malloc or C++ new. It allows you to
> allocate memory basically as you normally would, without explicitly
> deallocating memory that is no longer useful. The collector automatically
> recycles memory when it determines that it can no longer be otherwise
> accessed.

In pgcopydb C code base it's been hard to maintain proper malloc/free
concerns while also implementing proper error-handling, which is a common
problem when writing C code. In order to ease the maintenance of the code
and reduce production hazards, the best way to proceed looks like automating
the memory management altogether by using a Garbage Collector.

* Revert removal of regree() calls.

* Get rid of LinesBuffer.ownsBuffer.

Now that libgc is handling the memory for us, we don't need to keep track of
the ownership of the buffer anymore.
arajkumar added a commit to arajkumar/pgcopydb that referenced this pull request Mar 4, 2024
pgcopydb recently switched to Boehm-Demers-Weiser Garbage Collector[1] and replaced
malloc family with GC_malloc and friends. However, strdup is not replaced
to GC_strdup.

Solution: Use macros to replace strdup/strndup with it's GC variant.

[1] dimitri#647

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
dimitri pushed a commit that referenced this pull request Mar 4, 2024
pgcopydb recently switched to Boehm-Demers-Weiser Garbage Collector[1] and replaced
malloc family with GC_malloc and friends. However, strdup is not replaced
to GC_strdup.

Solution: Use macros to replace strdup/strndup with it's GC variant.

[1] #647

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERROR: corrupted size vs. prev_size
3 participants