Skip to content

Optimize JSON pretty print indentation performance#21474

Open
LamentXU123 wants to merge 3 commits intophp:masterfrom
LamentXU123:optimaze-1
Open

Optimize JSON pretty print indentation performance#21474
LamentXU123 wants to merge 3 commits intophp:masterfrom
LamentXU123:optimaze-1

Conversation

@LamentXU123
Copy link
Contributor

This PR optimizes the php_json_pretty_print_indent function in the JSON extension to improve performance when encoding structures with JSON_PRETTY_PRINT.

When I was reading this, I found that now, the indentation logic used a for loop to append spaces in increments of 4 characters per depth level. For a JSON structure with a depth of N, this resulted in N consecutive calls to smart_str_appendl. This approach introduces unnecessary overhead due to repeated function calls.

So I think I would introduce this space-time optimization to add a static constant string of pre-allocated spaces. For almost all typical JSON depths, this reduces the number of smart_str_appendl calls from O(N) to exactly 1. This significantly reduces function call overhead and improves CPU performance during json_encode with JSON_PRETTY_PRINT, especially for deeply nested data.

@staabm
Copy link
Contributor

staabm commented Mar 20, 2026

This significantly reduces function call overhead and improves CPU performance during json_encode with JSON_PRETTY_PRINT, especially for deeply nested data.

Could you give some before/after numbers?

@LamentXU123
Copy link
Contributor Author

This significantly reduces function call overhead and improves CPU performance during json_encode with JSON_PRETTY_PRINT, especially for deeply nested data.

Could you give some before/after numbers?

Sure do, but maybe tomorrow. I think this is a pretty obvious optimization so I don't do this initially

@iluuu1994
Copy link
Member

A short benchmark would be appreciated. smart_str_appendl() is inlined, so whether this actually improves performance is hard to predict.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants