decoder: Try new and faster approach to check if string needs to be r…

[JakubOnderka]

…eallocated

Summary by CodeRabbit

• Bug Fixes

  • Improved memory allocation handling when decoding JSON, including compatibility with custom memory managers.
  • Improved detection of available memory padding to help prevent decoding issues.

• Tests

  • Updated integration testing to build supported PHP versions directly and run tests in parallel with improved difference reporting.

[coderabbitai]

Warning

Review limit reached

@JakubOnderka, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c1b2dfd-1211-451b-aba4-0b26265beb30

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5ec02 and ed88190.

📒 Files selected for processing (1)

• .github/workflows/integration.yml

📝 Walkthrough

Walkthrough

The Ubuntu CI job now builds PHP and the extension directly, then runs tests in parallel. The decoder now handles custom memory managers explicitly and calculates standard allocator padding from Zend MM chunk boundaries.

Changes

Ubuntu CI build

Layer / File(s)	Summary
Direct Ubuntu build and test execution .github/workflows/integration.yml	The job installs build dependencies, compiles the matrix PHP version, builds the extension with phpize, and runs make test with parallel execution and diff reporting.

SIMDJSON allocation handling

Layer / File(s)	Summary
Zend memory padding calculation src/simdjson_decoder.cpp	simdjson_realloc_needed forces reallocation for custom memory managers and uses Zend MM chunk boundaries to detect available SIMDJSON padding. Persistent and permanent strings still require reallocation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• JakubOnderka/simdjson_php#23: Related changes in src/simdjson_decoder.cpp address SIMDJSON parser memory allocation and deallocation.
• JakubOnderka/simdjson_php#24: This PR modifies the Ubuntu development CI job introduced by that PR.
• JakubOnderka/simdjson_php#45: Both PRs modify .github/workflows/integration.yml, with the main PR refactoring the Ubuntu job execution strategy while PR #45 expands the testing matrix.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: a faster approach to determine whether a string needs reallocation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch updates-copilot

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR updates the decoder’s logic for deciding whether an input zend_string must be copied into a simdjson::padded_string by replacing an allocation-size-based check with a chunk-boundary-based check, aiming to reduce overhead in the fast path.

Changes:

• Reworks simdjson_realloc_needed() to infer padding safety from Zend MM chunk boundaries instead of calling zend_mem_block_size().
• Renames the helper’s parameter from json to str for clarity.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/integration.yml:
- Line 48: Update the tar invocation in the integration workflow to place the
stdin archive operand correctly while retaining extraction, xz decompression,
and --strip-components=1; ensure the piped curl stream is explicitly supplied as
tar’s archive file input rather than being misinterpreted as an option value.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b926dde7-511b-4eba-b8f1-40affe001493

📥 Commits

Reviewing files that changed from the base of the PR and between 7028eef and a7dd00b.

📒 Files selected for processing (2)

• .github/workflows/integration.yml
• src/simdjson_decoder.cpp

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/simdjson_decoder.cpp:166

• The new padding check computes remaining space based on the end of the current Zend MM chunk, not the actual allocation size of the zend_string. This can return "no realloc needed" even when the string’s allocation doesn’t include SIMDJSON_PADDING bytes, which risks out-of-bounds reads when creating a padded_string_view.

    // Get start position of chunk that stores zend_string
    size_t chunk_start = struct_addr & ~(ZEND_MM_CHUNK_SIZE - 1);
    size_t string_end = struct_addr + struct_size;
    // Check if chunk has still space for padding after string end
    return string_end + simdjson::SIMDJSON_PADDING > chunk_start + (struct_size / ZEND_MM_CHUNK_SIZE + 1) * ZEND_MM_CHUNK_SIZE;