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

Php service, PHP 7.2 support #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kalifg
Copy link

@kalifg kalifg commented Dec 4, 2018

Background

In the course of my day job, I needed to add base58 support but could not use the bcmath or gmp modules. To support that I wrote a pure PHP service.

To work with our environment I had to make a couple of changes to support PHP 7.2

Deficiencies

I'm aware it has (at least) the following issues:

  • Need to separate PHPService updates from PHP 7.2 compatibility updates
  • Need to make it PHP 5.3 compatible
    but I wanted to get discussion started so I can avoid repeating any other mistakes I have made

Performance

According to the included benchmarks, on my machine the pure PHP implementation is actually faster than the bcmath implementation:

StephenHill\Benchmarks\Base16Event
    Method Name    Iterations    Average Time      Ops/second
    ------------  ------------  --------------    -------------
    encodeBase16: [10,000    ] [0.0000000098705] [101,311,690.82126]
    decodeBase16: [10,000    ] [0.0000001375437] [7,270,417.75004]


StephenHill\Benchmarks\Base58BCMathEvent
    Method Name    Iterations    Average Time      Ops/second
    ------------  ------------  --------------    -------------
    encodeBase58: [10,000    ] [0.0000283010483] [35,334.38020]
    decodeBase58: [10,000    ] [0.0000288946390] [34,608.49604]


StephenHill\Benchmarks\Base58GMPEvent
    Method Name    Iterations    Average Time      Ops/second
    ------------  ------------  --------------    -------------
    encodeBase58: [10,000    ] [0.0000095309734] [104,921.07725]
    decodeBase58: [10,000    ] [0.0000168622017] [59,304.23668]


StephenHill\Benchmarks\Base58PHPEvent
    Method Name    Iterations    Average Time      Ops/second
    ------------  ------------  --------------    -------------
    encodeBase58: [10,000    ] [0.0000179829836] [55,608.12504]
    decodeBase58: [10,000    ] [0.0000192520618] [51,942.48845]


StephenHill\Benchmarks\Base64Event
    Method Name    Iterations    Average Time      Ops/second
    ------------  ------------  --------------    -------------
    encodeBase64: [10,000    ] [0.0000000110626] [90,394,482.75863]
    decodeBase64: [10,000    ] [0.0000000604630] [16,539,053.62776]

I have an idea that this is at least partially because I do a direct base change from 256 to 58, while the included services do 256->10->58 instead. I envision experimenting with altering the bcmath and gmp algorithms similarly to see if they also speed up.

bcmod returns a float, which isn't type-juggled to an int in
PHP 7.2
Uses hand-written pure PHP base-change algorithm
@stephen-hill stephen-hill added this to the v1.2 milestone Dec 4, 2018
@stephen-hill stephen-hill self-requested a review December 4, 2018 20:08
@stephen-hill stephen-hill removed this from the v1.2 milestone Dec 4, 2018
@stephen-hill
Copy link
Owner

Hi Michael

Thank you very much. This is awesome and very much appreciated.

I'll go over the code over the next couple of weeks - hopefully this weekend.

I'm very much up for going ahead with a v2.0 release which has a minimum PHP version requirement of 7.x (which version exactly, I'm happy to discuss). There are many good reasons to do this.

I will, however, cherry pick b2d6dc1 into the v1.1 branch, event though it isn't a PHP 5.x problem, it makes sense to me.

Anyway, great work, and I'll be back in touch soon.

Cheers
Stephen

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.

2 participants