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

tpie::priority_queue(mm_avail) not in documentation #244

Open
SSoelvsten opened this issue Jun 6, 2021 · 3 comments · May be fixed by #252
Open

tpie::priority_queue(mm_avail) not in documentation #244

SSoelvsten opened this issue Jun 6, 2021 · 3 comments · May be fixed by #252

Comments

@SSoelvsten
Copy link
Contributor

SSoelvsten commented Jun 6, 2021

There is a #ifndef DOXYGEN around the absolute memory constructor for tpie::priority_queue. Its implementation seems to not have anything dangerous in it since it starts with the following assertion.

assert(mm_avail <= get_memory_manager().limit() && mm_avail > 0);

Is it ill advised to use this function and hence should it not be part of the documentation?

@SSoelvsten
Copy link
Contributor Author

SSoelvsten commented Jun 6, 2021

My use my case/motivation is in the many algorithms of the BDD package Adiar that include lines close to the following:

// open `tpie::file_stream` for input and handle edge cases
. . .

// Init priority queues
tpie::memory_size_type available_memory = tpie::get_memory_manager().available();

tpie::priority_queue<...> pq_1(available_memory / 3);
tpie::priority_queue<...> pq_2(available_memory / 3);
tpie::priority_queue<...> pq_3(available_memory / 3);

// The actual algorithm
. . .

Initialising one after the other with a mere factor of 0.333 does not use all available memory as far as I can tell. I'd argue that writing the factors relative to each other (0.333, 0.5, 1.0) is not easy to understand/maintain. It especially becomes complicated, as pq_1 usually is a custom adiar::levelized_priority_queue that uses multiple tpie::merge_sorters and a single tpie::priority_queue (and hence needs to juggle both absolute and fractional memory).

@Mortal
Copy link
Collaborator

Mortal commented Jun 6, 2021

The constructor was hidden in commit f1d4d92 as part of a bigger commit that "fixes the documentation", whatever that means. I'm guessing I had problems getting doxygen to run, so I just hid it as a workaround. If there aren't any issues with building the documentation, then I think it's fine to just add it.

@SSoelvsten
Copy link
Contributor Author

I didn't think of putting this in could break compiling the documentation, so I have not tested that. I'll double check the ability to build the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants