Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create PHP RFC #8

Open
Renkas opened this issue Jan 15, 2020 · 10 comments
Open

Create PHP RFC #8

Renkas opened this issue Jan 15, 2020 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@Renkas
Copy link

Renkas commented Jan 15, 2020

Could you create RFC to maybe implement this into PHP? The speed imporovement is impressive and would improve PHP itself.

https://wiki.php.net/rfc

@crazyxman crazyxman added the help wanted Extra attention is needed label Aug 11, 2020
@kocsismate
Copy link

Hi @crazyxman,

I've just provided a PR to php-src in order to bundle your extension to the core: php/php-src#6551 and started a discussion about it: https://externals.io/message/112638

@enumag
Copy link

enumag commented Dec 31, 2020

In case it's not accepted into PHP would you consider making it available via PECL?

@kocsismate
Copy link

Yes, it's most probably won't be accepted just yet. But in this case, I'll try my best to make it available via PECL, sure.

@crazyxman
Copy link
Owner

@kocsismate thank you very much! It is a good idea to make it available through PECL

@sandrokeil
Copy link
Collaborator

@kocsismate I'm looking forward for your support to make it available via pecl and to improve this PHP extension.

@kocsismate
Copy link

In the light of the mailing list discussion (https://externals.io/message/112638), I abandon my proposal, as people would rather prefer to use libsimdjson as an optional JSON parsing backend.

@nemanjajojic
Copy link

do we have any news on progress regarding making it available through PECL?

@kocsismate
Copy link

People in the ML suggested to make simdjson available as a parsing backend for the built-in json-related functions, so if I tried to work on this, then I would rather follow this approach, instead of creating a new PECL extension.

@TysonAndre
Copy link
Collaborator

TysonAndre commented Dec 17, 2021

People in the ML suggested to make simdjson available as a parsing backend for the built-in json-related functions

That seems viable once implemented and the test cases of edge cases pass - if simdjson fails to parse it, then the exception should be cleared and json_decode() can fall back to the original json implementation so that https://www.php.net/json_last_error and JSON_THROW_ON_ERROR will work as expected and valid inputs would be permitted with flags such as JSON_BIGINT_AS_STRING

php > var_export(json_decode('[111111111111111111111111111111111111]', true, 10, JSON_BIGINT_AS_STRING));
array (
  0 => '111111111111111111111111111111111111',
)
php > var_export(simdjson_decode('[111111111111111111111111111111111111]'));
Warning: Uncaught RuntimeException: Problem while parsing a number in php shell code:1
Stack trace:
#0 php shell code(1): simdjson_decode('[11111111111111...')
#1 {main}
  thrown in php shell code on line 1

@TysonAndre
Copy link
Collaborator

TysonAndre commented Oct 18, 2022

The pecl simdjson 3.0.0 release should be in a good state (including for zts releases) for proposing to include as a configure option in php 8.3

This will speed up the happy path, where the json string is valid json (see previous comment for error handling).

  • Unsupported $flags can be skipped and use the original implementation. The ondemand C simdjson option seems like it would be slower than dom from the documentation but I haven't benchmarked it yet.
  • Edge cases for floating-point numbers should be consistent with the patched simdjson code
  • ZTS should work properly. Edge cases such as unicode surrogate pairs are now also consistent for error handling.
  • Depth handling when arrays do/don't have properties is off by one and will need to be handled in the fork of the C simdjson library
  • simdjson should be avoided for len > 4GB or depth > SOME_REASONABLE_C_STACK_DEPTH (e.g. have to account for smaller stacks in https://www.php.net/fiber - bison does not use the C stack)

I'd still need to fix #81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants