[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: commit: ldap/servers/slapd/back-bdb add.c cache.c

Howard Chu wrote:
hyc@OpenLDAP.org wrote: 
Update of /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb

Modified Files:
    add.c  1.160 -> 1.161
    cache.c  1.126 -> 1.127

Log Message:
Tweak bei_state so cache_lru_add doesn't ever try to free just-added
entries. This allows us to use the frontend's entry directly instead
of having to entry_dup it before adding to the cache.

I remember regretting having to use entry_dup here in the first place, 'way back when. Removing it shaves another minute off my test; the ldapadd that took an hour and 33 minutes now completes in just 5 minutes.

Of course this change affected a few other modules (ITS#4767, #4768). The entry in op->ora_e used to always be owned by the caller of be_add, so the caller would either free it unconditionally to cleanup or just allocate it from local/temporary memory. Now back-bdb/hdb takes ownership of the entry and zeroes out op->ora_e before returning, so the caller must always provide a normally allocated entry. The caller must also check for op->ora_e being non-NULL instead of freeing it unconditionally.

Currently the patches I wrote for the two ITS's instead check if op->ora_e == the orignally passed in entry, on the assumption that a response callback might change the entry after it's been added to the back-bdb entry cache. Frankly I can't think of any good reason for an overlay to change things at that point in time, so just checking for NULL ought to be sufficient.

--
  -- Howard Chu
  Chief Architect, Symas Corp.  http://www.symas.com
  Director, Highland Sun        http://highlandsun.com/hyc
  OpenLDAP Core Team            http://www.openldap.org/project/