feat: per-model locks for parallel model loading

[micmarty-deepsense]

Summary

Replace single global models_lock with per-model locks to enable parallel loading of different models.

Motivation

Current implementation serializes ALL model loading. In multi-model environments (detectron2 + yolox), this creates unnecessary bottleneck. Different models can safely load in parallel so that one partitioning request does not block others

We were seeing random partition_connection_error messages in the partitioner plugin logs, and later in the UI when a workflow finished.

Changes

• Replace models_lock with _models_locks dict and _models_locks_lock
• Hierarchical locking in get_model(): fast path → dict lock → per-model lock
• Add test for concurrent different-model loading with proper parallelism verification
• Document thread safety model and GIL dependency for Python 3.13+ awareness
• Bump version to 1.2.0

Thread Safety

• Lock dictionary protected by _models_locks_lock
• Per-model locks serialize same-model initialization
• Double-check pattern prevents duplicate loads
• CPython GIL provides atomicity for dict operations (documented for future Python versions)

[micmarty-deepsense]

@claude review

[claude]

Claude finished @micmarty-deepsense's task —— View job

---

Code Review Progress ✅

• Read repository guidelines and setup instructions
• Analyze the main implementation in unstructured_inference/models/base.py
• Review the test implementation and coverage
• Check version bump and changelog
• Assess thread safety and locking mechanism
• Provide comprehensive review feedback

---

Review Summary

This PR successfully implements per-model locks to replace the global models_lock, enabling parallel loading of different models. The implementation is well-designed and addresses a real bottleneck in multi-model environments.

✅ Strengths

1. Excellent Thread Safety Design

   • Hierarchical locking pattern is correctly implemented: fast path → dict lock → per-model lock
   • Double-check pattern in get_model() prevents race conditions (base.py:107-120)
   • Per-model locks serialize same-model initialization while allowing different models to load concurrently

2. Comprehensive Test Coverage

   • test_get_model_concurrent_different_models() excellently validates true parallelism with timing verification
   • Test confirms both models start before either finishes, proving parallel execution
   • Existing thread safety tests remain intact (test_get_model_threaded())

3. Performance Impact

   • Unbounded dictionary approach is appropriate for typical use cases (<100 models)
   • Memory overhead clearly documented: ~200 bytes per lock, ~20KB for 100 models
   • Future optimization path documented (lock striping for >1000 models)

4. Backward Compatibility

   • No API changes required
   • All existing functionality preserved

⚠️ Areas for Consideration

1. Documentation Excellence

   • Thread safety model clearly documented in docstring (base.py:23-29)
   • CPython GIL dependency properly documented for Python 3.13+ awareness
   • Good performance trade-off documentation (base.py:60-63)

2. Memory Management

   • Current implementation keeps all locks forever (unbounded growth)
   • For long-running processes with many unique model names, consider cleanup strategy
   • WeakValueDictionary correctly identified as unsuitable

3. Error Handling

   • Existing error handling maintained
   • Lock acquisition failures would propagate appropriately

📝 Minor Suggestions

1. Type Hints: Consider adding return type hint to get_model() function for consistency
2. Lock Naming: The _models_locks_lock could be renamed to _models_locks_mutex for clarity about its purpose

🔍 Code Quality Assessment

• Thread Safety: ✅ Excellent - proper hierarchical locking with double-check pattern
• Performance: ✅ Good - minimal overhead, significant parallelism gains
• Maintainability: ✅ Good - clear documentation and well-structured code
• Testing: ✅ Excellent - comprehensive tests including parallelism verification
• Backward Compatibility: ✅ Perfect - no breaking changes

Recommendation: ✅ APPROVE

This is a well-implemented solution that directly addresses the reported bottleneck. The thread safety design is robust, tests comprehensively verify the parallel behavior, and the documentation clearly explains the approach and trade-offs.

The version bump to 1.2.0 and changelog entry appropriately reflect this enhancement. This change should provide significant performance improvements in multi-model environments while maintaining full backward compatibility.

---