Skip to content

cffi perf: borrow-don't-own strings + dirent anchor index (drop getEntryByPath cost ~3×) #12

Description

@jasontitus

Profiling on real-world consumers (libzim-shim's bench harness against
700K-entry / 1.5 GB ZIMs) shows zimru_archive_get_entry_by_path at
3-5× slower than libzim's equivalent, and zimru_item_get_data
about 4× slower. Most of the gap is allocation overhead at the C
ABI surface, not anything fundamental about the readers.

Where the cost is

Every zimru_archive_get_entry_by_path allocates two CStrings
in Entry::box_entry:

// src/cffi/entry.rs
pub(crate) fn box_entry(&self, e: Entry) -> *mut zimru_entry_t {
    let path = CString::new(e.path()).unwrap_or_else(...);
    let title = CString::new(e.title()).unwrap_or_else(...);
    Box::into_raw(Box::new(zimru_entry_t { inner: e, path, title }))
}

zimru_item_get_data similarly allocates path + mimetype CStrings on
every call (src/cffi/item.rs).

A workload that looks up the same article 1M times allocates 4M
CStrings (entry's path + title, item's path + mimetype). On a server
serving popular articles that's the dominant cost.

Proposed redesign: borrow, don't own

Strings exposed by the C ABI become *const c_char whose lifetime is
tied to the archive, not the entry. The archive carries an
interning table mapping (namespace, url) -> CString; first lookup
of a given path inserts; subsequent lookups return the cached
pointer. Same trick for titles and mimetypes.

pub struct zimru_archive_t {
    inner: Archive,
    // Already exists as an unstructured Vec<CString>; replace with
    // a HashMap so repeat lookups don't re-intern.
    paths:     Mutex<HashMap<(u8, String), CString>>,
    titles:    Mutex<HashMap<(u8, String), CString>>,
    mimetypes: Mutex<HashMap<u16,           CString>>,  // by mime index
}

zimru_entry_t stops carrying owned CString fields and instead
holds borrowed pointers into the archive's tables. Callers' lifetime
contract: every string is valid until zimru_archive_close.

This is a C ABI redesign in the sense that the guarantees
change (callers must hold the archive open while they hold any
borrowed pointers), but the signatures don't, and the existing
shim code already does this correctly.

Other low-hanging perf items in the same area

These can land in the same PR or as follow-ups:

  1. Dirent anchor index for path lookup — every binary search
    currently starts at the midpoint of the URL pointer list and walks
    ~log₂(N) dirents from a cold start. With ~700 K entries that's ~20
    page touches per first-cold lookup. Building an array of (every
    1024th URL pointer's path → dirent index) on archive open turns
    the search into "narrow to a 1024-wide window, then binary search
    inside it" — log₂(1024) = 10 dirent touches max, all in a single
    page once the window is identified. ~60 KB extra memory on a 700 K
    archive, dramatic locality win.

  2. madvise(MADV_WILLNEED) on the URL pointer table at archive
    open. Single sequential read pages it in once instead of paying N
    page-faults across the first N lookups.

  3. mimetype: &str from MimeList::get is already a slice into
    the mmap (no allocation), but the C ABI converts it to String
    then CString per item. With the borrowed-pointer redesign this
    collapses to a one-time interning per mimetype index.

Acceptance

  • zimru_entry_t no longer owns CString fields for path/title;
    zimru_item_t ditto for path/mimetype. Borrowed pointers come
    from the archive's interning tables.
  • Lifetime contract documented in src/cffi/mod.rs preamble:
    "Strings remain valid until zimru_archive_close. Holding a
    borrowed pointer past that point is undefined behaviour."
  • tests/cffi_smoke.{c,cpp} regression-tests: open archive, look
    up an entry, free the entry, look up the same entry again
    (different handle), get_data on it, free everything. The borrow
    pattern is exercised end-to-end.
  • (Optional) microbench in bench/ that does 1M getEntryByPath
    calls on a real ZIM and reports per-op µs. Target: bring zimru
    within 1.5× of libzim (was 3-5×).
  • Dirent anchor index — separate commit if desired, same PR.
  • No regressions in default-feature build, reader-only build, or
    cffi smoke tests.

Coordination

Will likely require a small change in the shim's Archive PIMPL —
where it currently makes its own std::string copies "just in case",
those become unnecessary. Coordinate with libzim-shim's maintainer
before merging this so the shim-side update lands in lockstep.

Source

Perf doc items 1, 2, 5 from the libzim-shim project's
ZIMRU_PERF.md. Paraphrased here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions