Re: (ITS#8875) [Patch] Performance problems in back-mdb with large DITs and many aliases

[henrik.bohnenkamp@ionos.com]

On Thu, Apr 04, 2019 at 05:32:16PM +0100, Howard Chu wrote:
Hi Howard,

thanks for your comments. There is now a new version of patches available in my 
github repository. Apart from addressing your comments, I have rebased the patches against 
the current master branch (quite a lot activity in the last two months, I have noticed). 
In particular, the new function mdb_get_aliases  now also uses the global size variables
for IDL dimensions, rather than the previous CPP constants.

As for your comments:


> 
> Hi, thanks for the report and investigation. I've briefly reviewed your
> patches. Please squash your #3 into #2, there's no reason to preserve that.

done 

> 
> We don't use "inline" in this code. If the compiler is smart enough it will
> inline automatically anyway. Please drop that from your patch.

done

> 
> No need to keep the old mdb_idscope invocation commented out in patch #2.
> Just delete it, we have the git history.

done 

> 
> In patch #1 delete.c your patch line #145 is incorrect. You should be
> using is_entry_alias(e) instead of (op->ora_e.) The entry in question is not
> part of the op request, only its DN is in the Delete request.

Ah, that was a mistake when squashing my commits down to the two patches... patch 1 introduced
the faulty line with  op->ora_e, patch 2 changed op->ora_e to e. I have cleaned that up. 

> 
> Not sure why you dropped nsubs from the diskNode definition. Are you sure you
> haven't broken the nsubs processing in this patch?

Note that the nsubs declaration was always commented out and had thus only a decorative function. 
Anyway, I have added this comment back to reduce the foot print of the patches.


The current master branch with the two patches builds without problems. 
I have run the mdb tests in the tests directory... the last test was not applied due 
to missing software. All other 74 tests passed. (ubuntu 18.4). 

Best regards
Henrik
> 
> > 
--
Henrik Bohnenkamp