iotbx.cif.reader: zero-copy parse_file when engine="xcif" + file_path

When the reader is given a plain (uncompressed) file_path and
engine="xcif", dispatch to xcif_ext.parse_file (memory-mapped in C++)
instead of reading the file into a Python string and calling
xcif_ext.parse. Avoids a whole-file str() allocation on the hot path.

Binary detection runs via detect_binary_file.from_initial_block on
the first 1000 bytes, preserving the "Binary file detected" abort.

Compressed files (.gz/.Z/.bz2) and file_object= callers continue to
use the existing read-into-string path; parse_file cannot mmap a
compressed stream or an open file object.

Walker logic factored out of _drive_builder_from_xcif into a shared
_walk_xcif_doc helper; both parse/parse_file wrappers reuse it.

Tests verify file_path + xcif invokes parse_file (not parse) and
that the resulting model matches the input_string path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: olegsobolev

iotbx/cif/__init__.py

@@ -26,6 +26,7 @@
 from libtbx.utils import detect_binary_file
 from libtbx import smart_open
 
+import os
 import sys
 
 distances_as_cif_loop = geometry.distances_as_cif_loop
@@ -42,37 +43,29 @@ class CifParserError(Sorry):
 _VALID_ENGINES = ("ucif", "xcif")
 
 
-def _drive_builder_from_xcif(builder, input_string, strict):
-  """Parse input_string with xcif and drive the given cif_model_builder-style
-  builder via its callback methods (add_data_block / add_data_item / add_loop /
-  start_save_frame / end_save_frame).
+_XCIF_COMPRESSED_SUFFIXES = (".gz", ".Z", ".bz2")
 
-  Returns a (possibly empty) list of error message strings. xcif throws on
-  first parse error rather than accumulating, so the list has at most one
-  entry. When it does, the builder is left with whatever state it had
-  before the throw — which is nothing, since xcif_ext.parse runs to
-  completion before the walker touches the builder.
 
-  strict=False relaxes two things: pair items appearing before the first
-  data_ block are attached to an implicit 'global_' block; and explicit
-  'global_' block headers (STAR/DDL2 convention, used by the cctbx
-  monomer library) are accepted. strict=True rejects both.
+def _xcif_can_use_parse_file(file_path):
+  """True when file_path is safe to hand to xcif_ext.parse_file (which
+  memory-maps). False for compressed files; smart_open handles those,
+  parse_file does not."""
+  if file_path is None:
+    return False
+  return not file_path.endswith(_XCIF_COMPRESSED_SUFFIXES)
+
+
+def _walk_xcif_doc(builder, doc):
+  """Drive `builder` (a cif_model_builder-style object with
+  add_data_block / add_data_item / add_loop / start_save_frame /
+  end_save_frame) from an already-parsed xcif Document.
 
   The xcif C++ parser stores block and save-frame names with the data_/save_
   prefix stripped; the builder's strip-before-first-underscore logic
   (iotbx/cif/builders.py add_data_block / start_save_frame) expects raw
   tokens, so we prepend the prefix back.
   """
   import xcif_ext
-  try:
-    doc = xcif_ext.parse(input_string, strict=strict)
-  except (ValueError, RuntimeError) as e:
-    # xcif raises ValueError for std::invalid_argument (explicit translator)
-    # and RuntimeError for CifError (inherits std::runtime_error, default
-    # boost.python translation). Return the message so the caller can
-    # either surface it as CifParserError or stash it for error_count().
-    return [str(e)]
-
   # Use the enum values from xcif_ext directly — no ABI drift.
   # int(...) because bp::enum_ wraps values in a Python type;
   # the tuple comparisons below want plain ints.
@@ -119,6 +112,45 @@ def _emit_pair_or_loop(kind, idx, pair_tags, pair_values, loops):
         builder.end_save_frame()
       else:
         _emit_pair_or_loop(kind, idx, pair_tags, pair_values, loops)
+
+
+def _drive_builder_from_xcif(builder, input_string, strict):
+  """Parse input_string with xcif and drive the given builder.
+
+  Returns a (possibly empty) list of error message strings. xcif throws on
+  first parse error rather than accumulating, so the list has at most one
+  entry. When it does, the builder is left with whatever state it had
+  before the throw — which is nothing, since xcif_ext.parse runs to
+  completion before the walker touches the builder.
+
+  strict=False relaxes two things: pair items appearing before the first
+  data_ block are attached to an implicit 'global_' block; and explicit
+  'global_' block headers (STAR/DDL2 convention, used by the cctbx
+  monomer library) are accepted. strict=True rejects both.
+  """
+  import xcif_ext
+  try:
+    doc = xcif_ext.parse(input_string, strict=strict)
+  except (ValueError, RuntimeError) as e:
+    # xcif raises ValueError for std::invalid_argument (explicit translator)
+    # and RuntimeError for CifError (inherits std::runtime_error, default
+    # boost.python translation). Return the message so the caller can
+    # either surface it as CifParserError or stash it for error_count().
+    return [str(e)]
+  _walk_xcif_doc(builder, doc)
+  return []
+
+
+def _drive_builder_from_xcif_file(builder, file_path, strict):
+  """Like _drive_builder_from_xcif but dispatches to xcif_ext.parse_file
+  (memory-mapped, zero-copy). Avoids allocating a Python string copy of
+  the file contents when the caller supplied a plain uncompressed path."""
+  import xcif_ext
+  try:
+    doc = xcif_ext.parse_file(file_path, strict=strict)
+  except (ValueError, RuntimeError) as e:
+    return [str(e)]
+  _walk_xcif_doc(builder, doc)
   return []
 
 
@@ -146,6 +178,24 @@ def __init__(self,
     else: assert cif_object is None
     self.builder = builder
     self.original_arrays = None
+    self._xcif_errors = []
+    self.parser = None
+    # Fast path: xcif + plain uncompressed file_path -> memory-mapped
+    # parse_file, skipping the Python-string copy of the whole file.
+    # file_object= callers always go through the read-into-string path
+    # because there is no file descriptor to hand to mmap.
+    if (engine == "xcif"
+        and file_object is None
+        and file_path is not None
+        and _xcif_can_use_parse_file(file_path)):
+      resolved = os.path.expanduser(file_path)
+      if detect_binary_file.from_initial_block(resolved):
+        raise CifParserError("Binary file detected, aborting parsing.")
+      self._xcif_errors = _drive_builder_from_xcif_file(
+        self.builder, resolved, strict)
+      if raise_if_errors and self._xcif_errors:
+        raise CifParserError(self._xcif_errors[0])
+      return
     if file_path is not None:
       file_object = smart_open.for_reading(file_path)
     else:
@@ -159,11 +209,9 @@ def __init__(self,
       len(input_string), binary_detector.monitor_initial)
     if binary_detector.is_binary_file(block=input_string):
       raise CifParserError("Binary file detected, aborting parsing.")
-    self._xcif_errors = []
     if engine == "xcif":
       self._xcif_errors = _drive_builder_from_xcif(
         self.builder, input_string, strict)
-      self.parser = None
       if raise_if_errors and self._xcif_errors:
         raise CifParserError(self._xcif_errors[0])
     else:

xcif/regression/tst_iotbx_cif_reader_engine.py

@@ -3,13 +3,16 @@
 Covers:
 - Unknown engine values are rejected with ValueError before any file I/O.
 - Known engines ("ucif", "xcif") are accepted.
+- engine="xcif" with file_path dispatches to xcif_ext.parse_file (zero-copy
+  mmap) rather than reading the whole file into a Python string.
 """
 from __future__ import absolute_import, division, print_function
 
 import os
 import tempfile
 
 import iotbx.cif
+import xcif_ext
 
 
 def exercise_engine_validated_before_io():
@@ -47,9 +50,101 @@ def exercise_known_engines_accepted():
       "pair not parsed with engine=%s: got %r" % (engine, model["t"]["_k"])
 
 
+_SAMPLE_CIF = """\
+data_t
+_k v
+loop_
+_a.id
+_a.val
+ 1 x
+ 2 y
+"""
+
+
+def _write_sample(tmpdir):
+  p = os.path.join(tmpdir, "sample.cif")
+  with open(p, "w") as f:
+    f.write(_SAMPLE_CIF)
+  return p
+
+
+def exercise_file_path_uses_parse_file():
+  # Record which xcif_ext entry point the reader invokes.
+  real_parse = xcif_ext.parse
+  real_parse_file = xcif_ext.parse_file
+  calls = {"parse": 0, "parse_file": 0}
+
+  def _tracking_parse(text, **kw):
+    calls["parse"] += 1
+    return real_parse(text, **kw)
+
+  def _tracking_parse_file(path, **kw):
+    calls["parse_file"] += 1
+    return real_parse_file(path, **kw)
+
+  xcif_ext.parse = _tracking_parse
+  xcif_ext.parse_file = _tracking_parse_file
+  try:
+    tmpdir = tempfile.mkdtemp(prefix="xcif_reader_item5_")
+    try:
+      path = _write_sample(tmpdir)
+
+      # input_string + xcif: parse() is used, parse_file() is NOT.
+      iotbx.cif.reader(input_string=_SAMPLE_CIF, engine="xcif").model()
+      assert calls["parse"] == 1, \
+        "input_string path should call parse: %r" % calls
+      assert calls["parse_file"] == 0, \
+        "input_string path must NOT call parse_file: %r" % calls
+
+      # file_path + xcif: parse_file() is used, parse() is NOT.
+      calls["parse"] = 0
+      calls["parse_file"] = 0
+      iotbx.cif.reader(file_path=path, engine="xcif").model()
+      assert calls["parse_file"] == 1, \
+        "file_path path should call parse_file: %r" % calls
+      assert calls["parse"] == 0, \
+        "file_path path must NOT call parse (avoid Python-string copy): %r" % (
+          calls,)
+    finally:
+      import shutil
+      shutil.rmtree(tmpdir, ignore_errors=True)
+  finally:
+    xcif_ext.parse = real_parse
+    xcif_ext.parse_file = real_parse_file
+
+
+def exercise_file_path_matches_input_string():
+  tmpdir = tempfile.mkdtemp(prefix="xcif_reader_item5_")
+  try:
+    path = _write_sample(tmpdir)
+    m_str = iotbx.cif.reader(
+      input_string=_SAMPLE_CIF, engine="xcif").model()
+    m_file = iotbx.cif.reader(file_path=path, engine="xcif").model()
+    # Compare block set, pair items, loop tag+content.
+    assert list(m_str.keys()) == list(m_file.keys())
+    for name in m_str:
+      b_s = m_str[name]
+      b_f = m_file[name]
+      assert list(b_s._set) == list(b_f._set), \
+        "source-order drift between input_string and file_path"
+      assert dict(b_s._items) == dict(b_f._items)
+      assert set(b_s.loops.keys()) == set(b_f.loops.keys())
+      for ln in b_s.loops:
+        lp_s = b_s.loops[ln]
+        lp_f = b_f.loops[ln]
+        assert set(lp_s.keys()) == set(lp_f.keys())
+        for tag in lp_s.keys():
+          assert list(lp_s[tag]) == list(lp_f[tag])
+  finally:
+    import shutil
+    shutil.rmtree(tmpdir, ignore_errors=True)
+
+
 def run():
   exercise_engine_validated_before_io()
   exercise_known_engines_accepted()
+  exercise_file_path_uses_parse_file()
+  exercise_file_path_matches_input_string()
   print("OK")