[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