Fix multiple errors in Chia related changes to relic if ALLOC!=AUTO #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are mostly compilation errors and a few fixes for memory leaks.
|
ALLOC dynamic and other types of allocation are going to be removed eventually in relic, so I didn't try to get it to work with those (only auto). Using other allocs requires calling ep_new, ep_null, ep_free, etc, so it also clutters up the code a lot. In ALLOC=AUTO, most of these operations actually do nothing at all, so I just omitted them to keep the code cleaner. bn_new is an exception, in that it actually has to get called. Anyway, I think these changes make sense to keep the relic code consistent. |
UdjinM6
pushed a commit
to UdjinM6/bls-signatures
that referenced
this pull request
Jun 30, 2022
…twork#36) * refactor: use runtime.KeepAlive to prevent GC memory release * refactor: change a way how CGO links deps, add gmp as required dep * fix: update build-test.yaml and Makefile * fix: update Makefile * fix: update Makefile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are mostly compilation errors and a few fixes for memory leaks.
I encountered these while trying to build and test the library with ALLOC=DYNAMIC. This PR only fixes the issues which I found in the relic source. There are much more issues with the BLS library, but I'm not going to create a PR for those...these required much more changes. And instead of fixing the issues on that level, I'd suggest to implement small C++ wrappers around relic::bn_t, relic::ep_t and relic::ep2_t which do the appropriate relic::xxx_new/relic::xxx_free calls in constructors/destructors. This way you'd catch all possible memory leaks and clean up the code as well.
I decided to create this PR so that whenever you decide to backport the
ep_maprelated changes to upstream, these changes are included.And FYI (not really related to this PR), the tests I did with ALLOC=DYNAMIC were a dead end. As expected, it got slower in single threaded environment, but my hope was that it would improve multithreaded environments, where currently parallel verification does not scale after about 4 threads on my system (with 8 cores). I assume that the issues I have with multithreading are in some way related to L1/L2 cache issues and as I had no idea how to fix this, I gave ALLOC=DYNAMIC a try to compare it ;)