Skip to content

Conversation

@CybotTM
Copy link
Contributor

@CybotTM CybotTM commented Jan 23, 2026

Summary

Optimizes document entry lookups in ProjectNode from O(n) iteration to O(1) hash-based lookups.

This PR contains 3 distinct commits, each addressing a specific concern:

1. Preparatory Refactor: File path comparison (4 files)

Replaces object identity comparison (===) with file path comparison when matching DocumentEntryNode instances.

Why this matters:

  • Object identity fails when entries are deserialized from cache (new instances)
  • File path is the natural unique identifier for documents
  • Enables the O(1) optimization in commit 2

Changed files:

  • GlobalMenuPass.php - root document lookup
  • ToctreeValidationPass.php - root document check
  • RenderContext.php - getDocumentNodeForEntry()
  • DocumentTreeIterator.php - current()

2. Performance Improvement: O(1) caching (1 file)

Adds hash-based O(1) lookups to ProjectNode:

// Before: O(n) iteration
foreach ($this->documentEntries as $entry) {
    if ($entry->getFile() === $file) return $entry;
}

// After: O(1) hash lookup
return $this->documentEntries[$file];

Changes to ProjectNode.php:

  • Cache root document entry for instant getRootDocumentEntry()
  • Use direct hash lookup in getDocumentEntry() by file path
  • Re-key array in setDocumentEntries() via addDocumentEntry()
  • Invalidate cache on reset()
  • Update type annotation: DocumentEntryNode[]array<string, DocumentEntryNode>

3. Test Adaptation (1 file)

Updates FunctionalTest.php to use withIsRoot(true) instead of manually creating a DocumentEntryNode that gets overwritten by the compiler.

Performance Impact

Method Before After
getRootDocumentEntry() O(n) O(1)
getDocumentEntry($file) O(n) O(1)

This is particularly impactful for large documentation projects with hundreds or thousands of documents.

See Performance Analysis Report for detailed benchmarks.

Test Plan

  • All existing tests pass
  • Code style checks pass for changed files
  • No breaking changes to public API

Related PRs

PR Description Status
#1287 Rendering caching layer Independent
#1288 RST parsing optimizations Independent
#1289 CLI container caching Independent
#1291 Symfony 8 compatibility ✅ Merged
#1293 This PR - ProjectNode O(1) document lookup

All PRs can be merged independently in any order.

Replace object identity comparison (===) with file path comparison
when matching DocumentEntryNode instances. The file path is the
natural unique identifier for a document entry.

This change:
- Makes the code more robust to serialization/deserialization
  (cached objects create new instances)
- Aligns with how SectionEntryNode comparison already works
- Enables future optimizations that may cache or recreate entry objects
- Is a prerequisite for O(1) lookup caching in ProjectNode

Changed locations:
- GlobalMenuPass: root document lookup
- ToctreeValidationPass: root document check
- RenderContext: getDocumentNodeForEntry()
- DocumentTreeIterator: current()

No behavior change - documents are still matched by their file path,
which was always the semantic intent.
Replace O(n) iteration with O(1) hash-based lookups for document
entry retrieval in ProjectNode.

Changes:
- Cache root document entry for instant getRootDocumentEntry()
- Use direct hash lookup in getDocumentEntry() by file path
- Properly rekey array in setDocumentEntries() via addDocumentEntry()
- Invalidate cache on reset()
- Update type annotation to array<string, DocumentEntryNode>

Performance impact:
- getRootDocumentEntry(): O(n) -> O(1)
- getDocumentEntry(): O(n) -> O(1)

This optimization is enabled by the previous commit's refactoring
from object identity to file path comparison, which ensures lookups
work correctly even with cached/deserialized document entries.
Update FunctionalTest to mark the document as root using withIsRoot(true)
before compilation, rather than pre-creating a DocumentEntryNode that
gets overwritten by the compiler.

This is the correct approach because:
- The compiler creates DocumentEntryNode during compilation
- Pre-creating an entry that gets replaced is misleading
- withIsRoot(true) signals intent to the compiler

Removed unused imports: DocumentEntryNode, TitleNode
@jaapio jaapio merged commit 8bba205 into phpDocumentor:main Jan 24, 2026
58 checks passed
@jaapio
Copy link
Member

jaapio commented Jan 24, 2026

Thanks, this makes a lot of sense.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants