Skip to content

Conversation

@ssoelvsten
Copy link
Contributor

@ssoelvsten ssoelvsten commented May 24, 2022

Closes #250 as a simpler (and working) alternative to #253 . All tests pass, and the computed value seems to be slightly above the min_m1 computed in calculate_parameters for a block size of 2, 4, 8, ..., 64. Except for the tests and my own project, Adiar, I do not know what to test it on.

@freekvw
Copy link
Collaborator

freekvw commented Jun 3, 2022

See #255. Not sure if this also solves your issue?

@ssoelvsten
Copy link
Contributor Author

ssoelvsten commented Jun 3, 2022

I tried to rerun the minimal example in #250 on main after the merge of #255 and I get

Not enough phase 1 memory for 128 KB items and an open stream! (25167632 < 25298776)

So, this function still seems to return a too small value. The computation in minimum_memory_phase1 is a separate code-duplication of the line you changed. Here, the only difference is the 128*1024 / m_item_size (now std::max(128*1024UL, 16*m_item_size)) is replaced with a 1 which is very unlikely to be the same.

@ssoelvsten ssoelvsten force-pushed the merge_sorter/min_memory_available branch 3 times, most recently from 25adb86 to 06a8a35 Compare June 3, 2022 13:05
@ssoelvsten ssoelvsten force-pushed the merge_sorter/min_memory_available branch from 06a8a35 to d587f0d Compare March 8, 2023 16:17
@ssoelvsten ssoelvsten force-pushed the merge_sorter/min_memory_available branch from d587f0d to 742f060 Compare March 9, 2023 15:37
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.

merge_sorter::minimum_memory_phase_X() is lower than actual memory requirements

2 participants