[PATCH][RFC] Make Samba faster by removing o(n) search in ltdb_is_indexed

Andrew Bartlett abartlet at samba.org
Thu Mar 9 09:16:29 UTC 2017


On Thu, 2017-03-09 at 09:23 +0100, Stefan Metzmacher wrote:
> Hi Andrew,
> 
> > This patch appears to address a performance issue during search
> > and modify attempting to determine if an attribute is in an index.
> > 
> > Our ldb_schema_attribute_by_name() in Samba uses a binary search
> > on the schema, and we now set the index flag there in that case.
> > 
> > Otherwise we fall back to a scan over the index list (no attempt is
> > made to try and merge the index values and attribute values in the
> > general case). 
> > 
> > Douglas will do some perf tests on this soon.
> 
> Can you please split the patch
> - First split out ltdb_index_load() without changing the logic
> - Use !ltdb->cache->attribute_indexes instead of
>   ltdb->cache->indexlist->num_elements == 0
> - change the arguments to ltdb_is_indexed()
> - implement the new LDB_ATTR_FLAG_INDEXED related logic
> - change samba to use it
> 
> While the argument change to ltdb_is_indexed()
> will work, I'd actually try to avoid passing
> const struct ldb_message *index_list to
> ltdb_is_indexed and it's callers.
> 
> We should not pass ltdb->cache->indexlist,
> I'm wondering if we could just pass 'module' down
> and get
> struct ltdb_private *ltdb =
> talloc_get_type(ldb_module_get_private(module), struct ltdb_private);
> and
> struct ldb_context *ldb = ldb_module_get_ctx(module);
> in ltdb_is_indexed().

Indeed.  :-)

> Or if it's faster we pass 'ldb' and 'ltdb' to ltdb_is_indexed()
> and let the caller get it out of 'module', because some caller
> seem to also need them.
> 
> I'm also wondering if the optimized code will always
> generate the same result as the other one.
> What if samba starts to pass LDB_ATTR_FLAG_INDEXED for an
> attribute that was not indexed before?

The list is synchronised by writing to the DB when the schema is
loaded, thankfully. 

> Are there other potential problems, where things can get out of sync?
> 
> At the end please let me push it to master, in order to make a
> separate
> "ldb: version ..." commit.

Thanks, that all sounds very reasonable.  I think my second version of
the patch does much as you describe, the rest shouldn't be hard at all.

Thanks!

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba




More information about the samba-technical mailing list