This change, along with the fixes in #49222 passes all ticket tests with 100% success.

PS: Before you say it Mark, I think the patches to re-tab the source tree are coming in the next week, so please don't be strict on my tab vs spaces :)

PS: Before you say it Mark, I think the patches to re-tab the source tree are coming in the next week, so please don't be strict on my tab vs spaces :)

Yeah man it's bad. Java style comments, inconsistent spacing/tabs. The patch itself looks good, but I'm not giving an ack until you provide the patch to fix all the spacing/tab/comment issues.

Also did you do any testing to see how the server process size looks (before and after applying your patch)? And what about a sanity ASAN test?

Thanks!

Java style comments are easy to fix, whitespace, no so much. There is a lot of change in there...

I haven't re-shaped the pblock yet, so I expect memory usage is the same still. Also, my builds are always asan, so this 100% passes ticket tests with asan enabled the entire duration. I went hardcore on this one :)

How would you consider a second patch on top of this that fixes the whitespacing?

How would you consider a second patch on top of this that fixes the whitespacing?

A second patch is fine, but I want them committed at the same time. And what I really want is the tab vs space inconsistencies fixed. For example, in some places you add code that uses tabs, but the surrounding code uses whites spaces, or you use white spaces when the surrounding code uses tabs. So you are technically adding to the problems we have in source code. I don't want to keep making the problem worse.

So I'll keep an eye out for your new patch. Seriously, it should only take you 10 or15 minutes to fix it up.

Yep, I was planning to merge both at the same time. I'll start on this now :) I just prefer to keep whitespace fixes seperate to "code fixes" is all.

TBH, Is it time we just ran clangformat and fixed this to spaces to end the problem once and for all? This is why I was asking about branching in 389-devel.

Thanks again mate :)

This patch applies "in addition to" and fixes the white space issues. If you want, I can squash the two together for the merge?

Thank you William! You don't need to squash them - two commits is fine.

commit 74c666b
commit eacb3c9
To ssh://git@pagure.io/389-ds-base.git
5680cc1..eacb3c9 master -> master

This is the final patch in the series. This converts the pblock to a tree of structs, that are allocated as needed.

Heap profiling shows the output:

Before:
bytes allocated in total (ignoring deallocations): 436.90MB (3.41MB/s)
After:
bytes allocated in total (ignoring deallocations): 374.80MB (2.87MB/s)

This is a 15% reduction in heap allocations, and the maximum size of any structure in the tree drops from 712 bytes to 200 bytes (slapi_pblock_intop) with the root pblock now being 88 bytes. There is some room for further refinement especially in the internal search use case. This may have significant effects on reducing our memory fragmentation.

Like other patches in the series, this has been run with ASAN and test cases. Most importantly, was the psearch test case to prevent regressions there (as I previously introduced).

PS: @mreynolds there is a second patch to follow to re-whitespace the file.

And the second patch that fixes the whitespace

Looks good!

commit 407d7d9
commit 7995007
To ssh://git@pagure.io/389-ds-base.git
a96c91d..7995007 master -> master

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2156

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.