Implemented protobuf extension

[huzhiguang]

Summary: Implemented protobuf extension.

php extension git address:https://github.com/allegro/php-protobuf

Generate php protobuf files you need to run using HHVM,e.g:

hhvm protoc-gen-php.php foo.proto

[huzhiguang]

@fredemmott @mxw Can you help review this extension?

[fredemmott]

I'm not going to be able to look at this really. That said:

• the reader/writer stuff doesn't belong in the HPHP namespace
• the f_printf thing is very unclear
• zend tests are only for tests copied directly from PHP itself
• there's an alternative implementation over at https://github.com/quizlet/protobuf/tree/hhvm - we should be comparing to that, not reviewing in isolation - cc @RyanGordon

[fredemmott]

also cc @binliu19

[RyanGordon]

Just FYI, I actually haven't been able to make any progress on that and because the pure-PHP Protobuf library works on HHVM (I've been maintaining it for HHVM compat and merging patches in the latest protobuf master as needed) so we haven't prioritized work on it. The JIT optimizes the pure-PHP Protobuf library pretty well but I'm sure this C implementation would still be several ms faster.

Since this is the furthest implementation of protobuf for HHVM, I would be happy to help test it and what not @huzhiguang

Also, are there any preferences about contributing this to the google/protobuf repository versus baked directly in HHVM? It would be nice if it was just built in directly but if it was merged in to the google repo it would probably get more maintenance. Thoughts?

[RyanGordon]

Also, I just looked through and this seems to only implement a smalls subset of the Protobuf extension. Is that correct?

[fredemmott]

Are there significant advantages to having an extension for this instead of using the PHP library? In many cases, C++ extensions end up being slower than JITed PHP.

[RyanGordon]

Ah interesting; The main performance problem we see in the PHP Protobuf library (without JIT) is that for every single request these functions listed here are called over and over again and are very expensive: https://github.com/google/protobuf/search?l=PHP&q=initOnce%28&type=&utf8=%E2%9C%93

With the JIT on these become significantly faster but there is still an impact. I think the Protobuf PHP C extension actually only initializes those things once for the duration of the PHP process daemon rather than for every request

[huzhiguang]

@fredemmott

• the reader/writer stuff doesn't belong in the HPHP namespace

  • re: I will add HPHP namespace to reader/writer

• the f_printf thing is very unclear

  • re:I will optimize print mode instend of f_printf

• zend tests are only for tests copied directly from PHP itself

  • re:I will add more test code

• there's an alternative implementation over at https://github.com/quizlet/protobuf/tree/hhvm - we should be comparing to that, not reviewing in isolation - cc @RyanGordon

  • re:
    • We have a variety of protobuf extensions in 2014，the test data content is online data
    • Test the two extensions: one C extension（so: https://github.com/allegro/php-protobuf） and the other is the PHP extension（phplib: https://github.com/drslump/Protobuf-PHP）
    • test data

• Protobuf avg data：

Engine+extension	unpack（avg）	pack（avg）	total
hhvm 3.0 + phplib	0.9ms	3.6ms	4.5ms
php 5.4 + phplib	5.4ms	2.8ms	8.2ms
php 5.2 + so	0.27ms	0.07ms	0.34ms

• Protobuf max and mix data：

Engine+extension	Min unpack	Max unpack	Min pack	Max pack
hhvm 3.0 + phplib	0.14ms	15.7ms	0.26ms	6.6ms
php 5.4 + phplib	0.9ms	23ms	0.3ms	4.5ms
php 5.2 + so	0.084ms	0.45ms	0.0097ms	0.14ms

Test data proves that C extension performance is better than php jited extension

[fredemmott]

Leaving the bulk for others (and I'll follow-up to make sure that happens).

Performance comparisons against a more recent version would likely be useful,

zend tests are only for tests copied directly from PHP itself

re:I will add more test code

Remove from zend/, put in slow/

[huzhiguang]

Performance comparisons against a more recent version would likely be useful,

I will follow this comparison test and give the data.

Remove from zend/, put in slow/

I will modify the code and PR it to you

[facebook-github-bot]

@binliu19 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.