Support disabling the Erlang allocator - #357
Conversation
| sqlite3_config(SQLITE_CONFIG_GETMALLOC, &default_alloc_methods); | ||
| if (erlang_allocator_enabled) { | ||
| sqlite3_config(SQLITE_CONFIG_MALLOC, &methods); | ||
| } |
There was a problem hiding this comment.
It seemed simpler to me to unconditionally load and restore the default allocator and only conditionally override the default allocator with the Erlang ones to reduce the amount of branching that needs to happen.
There was a problem hiding this comment.
Oh this is far simpler than what I had originally envisioned. I'll take a closer look at this tomorrow. As it stands I don't see a problem with this.
|
@warmwaffles I'm a little unsure about the testing story here because it doesn't seem like the existing allocator is tested currently. Adding the |
IIRC when I implemented it, it was not tested at all. I could not come up with a reasonable way to test it other than just running the library. However, I do think perhaps running the tests twice would be a solution. One runs with the custom allocator and one without it. |
|
Don't worry trying to figure out how to test the allocator being swapped. I'll spin up a separate task where this is configured to be true or false and run it on CI just to make sure nothing insane is being done with either allocator. |
|
Released under |
Allow disabling the Erlang allocator which can sometimes have worse performance characteristics compared to the default one as demonstrated in #356.
In addition to the added automated tests, I manually tested this against the repro file provided in the issue by setting my fork as a dependency and toggling the
disable_erlang_allocatorsetting.