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

Implement an allocator that can reuse allocated chunks. #3

Open
wants to merge 5 commits into
base: mujin
Choose a base branch
from

Conversation

ziyan
Copy link
Member

@ziyan ziyan commented Apr 29, 2022

Improve the memory pool allocator to reuse its allocated chunks.

  1. keep chunks in a doubly linked list
  2. on Clear(), set size to 0 for all chunks
  3. when allocating space, reuse previously allocated chunk
  4. when allocating larger space than previous chunks, find or allocate next available chunk and reorder the doubly linked list to reduce waste (I assume this rarely rarely happens, default chunk size is 64kB)

Assumption: Malloc() requested size is smaller than default chunk size in majority of cases.

Small program to demonstrate

#include <fstream>
#include <rapidjson/document.h>
#include <rapidjson/istreamwrapper.h>

int main() {
    std::ifstream ifs("bigfile.json");
    rapidjson::Document doc;
    for (size_t i = 0; i < 1000; i++) {
        // seek back to the beginning of the stream
        ifs.clear();
        ifs.seekg(0, std::ios::beg);
        rapidjson::IStreamWrapper isw(ifs);

        // reset the doc, or use a secondary doc and swap like flip-flop when parsing
        doc.SetNull();
        doc.GetAllocator().Clear();

        // parse
        doc.ParseStream<rapidjson::kParseFullPrecisionFlag>(isw);
    }
}

g++ -o testrapidjsonparse testrapidjsonparse.cpp

Size of the bigfile.json is 1.9MB. Loading it 1000 times only resulted in 33 allocation calls in the first parse.

heaptrack stats:
	allocations:          	33
	leaked allocations:   	0
	temporary allocations:	1

Screenshot from 2022-04-29 09-19-10

Before this PR

heaptrack stats:
	allocations:          	37008
	leaked allocations:   	0
	temporary allocations:	15000

@ziyan ziyan requested a review from rdiankov April 29, 2022 00:21
@ziyan ziyan changed the base branch from master to mujin April 29, 2022 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant