Hoist per property type meta out of the object loops by veewee · Pull Request #78 · php-soap/encoding

veewee · 2026-10-15T10:11:40Z

For every element it decodes or encodes, ObjectEncoder asks each property's type meta whether it is an attribute and, on decode, whether it is a list. Each answer is an Option that gets unwrapped inside a closure called through map_with_key. The answers only depend on the type, so ObjectAccess::build() now works them out once per type, and both loops in ObjectEncoder use a plain foreach over the precomputed values. The output does not change.

ObjectAccess gets two extra constructor arguments. Nothing outside the package builds one, so I took the constructor change for 0.x.

3MB synthetic workloads, seconds, median of 3 interleaved rounds:

workload	main	this PR
decode sparse, stdClass	0.713	0.478 (-33%)
decode sparse, classmap	1.139	0.909 (-20%)
decode dense, stdClass	0.298	0.266 (-11%)
decode dense, classmap	0.360	0.325 (-10%)
encode sparse, stdClass	1.698	1.413 (-17%)
encode sparse, classmap	1.715	1.531 (-11%)
encode dense, stdClass	0.734	0.704 (-4%)
encode dense, classmap	0.734	0.739 (+1%)

The phpbench subjects use small objects. Literal decode there is 4% to 7% faster, and the rest stays within the noise of the ext-soap subjects, which this change does not touch.

Summary by CodeRabbit

Bug Fixes
- XML decoding now returns an empty list when repeating elements are absent, while preserving attribute values alongside repeated elements.
- XML encoding now handles attributes alongside repeated elements and omits attributes with null values.

ObjectAccess now precomputes whether each property is an attribute and which default an absent element decodes to, so decoding no longer reads the type meta for every property of every object it builds. The decode loop is a plain foreach instead of map_with_key with a closure.

Encoding reads the precomputed isAttribute flag from ObjectAccess instead of the property type meta, and builds the child writers in a plain foreach instead of map_with_key with a closure.

coderabbitai · 2026-10-15T10:11:58Z

Warning

Review limit reached

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

Next included review available in 13 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: e3941e6f-bdfe-4c5a-9353-a63b101ab14a

📥 Commits

Reviewing files that changed from the base of the PR and between 1e7c814 and df58aa7.

📒 Files selected for processing: 3

src/Encoder/ObjectAccess.php
src/Encoder/ObjectEncoder.php
tests/Unit/Encoder/ObjectEncoderTest.php

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 75397c75-d942-4e9f-86b9-c421bf42b24b

📥 Commits

Reviewing files that changed from the base of the PR and between 16eead4 and 1e7c814.

📒 Files selected for processing: 3

src/Encoder/ObjectAccess.php
src/Encoder/ObjectEncoder.php
tests/Unit/Encoder/ObjectEncoderTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

ObjectAccess now stores per-property attribute flags and decode defaults. ObjectEncoder uses this metadata for XML encoding and decoding. Tests cover absent repeated elements and attribute handling alongside repeated elements.

Changes: Object XML encoding

Layer / File(s)	Summary
Build property metadata `src/Encoder/ObjectAccess.php`	`ObjectAccess` records whether each property is an attribute and assigns `[]` as the decode default for list properties, or `null` otherwise.
Use metadata in encoding and decoding `src/Encoder/ObjectEncoder.php`, `tests/Unit/Encoder/ObjectEncoderTest.php`	`ObjectEncoder` uses the stored attribute flags and decode defaults. Tests cover absent repeated elements, attribute handling, and repeated elements during encoding and decoding.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 1e7c8

No concrete issue in the changed encoding and decoding paths remains to block merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately describes the main change: precomputing per-property metadata outside the object encoding and decoding loops.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

The PossiblyNullArgument suppression only applies to attribute isos, which accept null, so it now sits on that branch alone.

The if only narrowed the null argument suppression; one match per property reads better.

veewee added 2 commits October 15, 2026 11:50

Use the hoisted attribute flag in the object encode loop

1e7c814

Encoding reads the precomputed isAttribute flag from ObjectAccess instead of the property type meta, and builds the child writers in a plain foreach instead of map_with_key with a closure.

veewee added 5 commits October 15, 2026 12:13

Cover the text value branch and scope the null argument suppression

1bcab50

The PossiblyNullArgument suppression only applies to attribute isos, which accept null, so it now sits on that branch alone.

Rename decodeDefaults to decodeDefaultValues

2e366a4

Describe what ObjectAccess holds per property

9adeac8

Decode each property through one match again

e6b19a2

The if only narrowed the null argument suppression; one match per property reads better.

Give the absent value its own match arm

df58aa7

veewee merged commit 403a479 into php-soap:main Oct 15, 2026
13 checks passed

veewee mentioned this pull request Oct 15, 2026

Leave out absent optional nillable elements on encode #79

Merged

veewee deleted the perf-hoist-type-meta branch October 16, 2026 08:49

Navigation Menu

Repository navigation

Hoist per property type meta out of the object loops - #78

Hoist per property type meta out of the object loops#78
veewee merged 7 commits into
php-soap:mainfrom
veewee:perf-hoist-type-meta

veewee commented Oct 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 15, 2026 •

edited

Loading

Review limit reached

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

veewee commented Oct 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

veewee commented Oct 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 15, 2026 •

edited

Loading