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

Configurable bit length #36

Merged
merged 13 commits into from
Mar 5, 2018

Conversation

courtney-miles
Copy link
Contributor

This PR is in regards to #35

Currently Optimus only supports encoding of integers up to 2^31-1 (e.g. 0 to 2147483647). This PR allows you to choose a different range for Optimus to use.

This is done by specifying a bit length, where the default bit length is 31, (to preserve current behaviour.) For example, the following will setup Optimus for 48 bit integers (e.g. 0 to 281474976710655).

$ php bin/optimus spark --bits 48
Prime: 59927384369137
Inverse: 166258800519441
Random: 3091726089
Bits: 48

    new Optimus(59927384369137, 166258800519441, 3091726089, 48);

The method for encoding values only works where the maximum integer is a power of 2, which is why the argument for initialising Optimus is bits rather than a maximum integer. (Please correct me if I'm wrong about the maths Optimus uses not working with arbitrary integer ranges.)

On the command line you cannot specify a bit length of 63 or higher -- the system breaks with numbers that large.

The one change I would like to make, but have not to preserve current logic, is for Optimus::encode() to throw an exception if you attempt to encode an integer greater that the bit length would allow. Or at least trigger a warning. This is because people may be encoding values without realising they may decode to a different value.

If the PR is tentatively accepted, I will also update the README.md.

I'm very open to any suggested revisions, or suggestion for a completely different strategy.

* Add tests for calculating inverse from a prime of types int or BigInteger
* Add test for exception if GMP extension not loaded when needed.
 and do not include uncovered files in coverage.
@courtney-miles
Copy link
Contributor Author

@jenssegers I would love to hear an initial impression of this.

I desperately need to get a solution for larger ints into a commercial product, and I'm happy to use my own fork. But I'm just looking for a little peace of mind before then.

@jenssegers
Copy link
Owner

Will check it out tomorrow. Your help is very much appreciated!

@courtney-miles
Copy link
Contributor Author

Note that I have removed myclabs from the exludes in box.json because the resulting phar file was generating the fofllowing error.

PHP Warning:  require(phar:///var/www/optimus/vendor/composer/../myclabs/deep-copy/src/DeepCopy/deep_copy.php): failed to open stream: phar error: "vendor/myclabs/deep-copy/src/DeepCopy/deep_copy.php" is not a file in phar "/var/www/optimus" in phar:///var/www/optimus/vendor/composer/autoload_real.php on line 66
PHP Fatal error:  require(): Failed opening required 'phar:///var/www/optimus/vendor/composer/../myclabs/deep-copy/src/DeepCopy/deep_copy.php' (include_path='.:/usr/share/php') in phar:///var/www/optimus/vendor/composer/autoload_real.php on line 66

I did not investigate what changes have lead to myclabs now being required for the optimus.phar file.

@jenssegers jenssegers merged commit af872b1 into jenssegers:master Mar 5, 2018

$doMode = $this->mode;

if ($doMode === self::MODE_NATIVE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed @courtney-miles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean as opposed to always using native or always gmp, rather than auto-switching to gmp on 64 bit systems only when dealing with the multiplication of large integers?

It could, in the constructor, force the mode to gmp when dealing with more than 31 bits for 64 bit numbers.

It was a speed decision to not use GMP if the specific values being multiplied do not need it. I was tempted to not do this because it makes the system more complicated.

I don't have the exact numbers now, but without GMP I was hashing X number of values in a couple of seconds, whilst with GMP that same number of values took 30 seconds.

This speed difference is not important to me and I'm happy to simplify and instead force GMP always when the bit length is greater than 31.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought gmp was faster at handling big numbers.

Also; would it be possible to pass a random max int instead of a bit length? Eg. I want max int to be 10.000.000. Or will that not work with the maths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought gmp was faster at handling big numbers.

It can handle bigger numbers than native PHP, but it seems if a number can be handled by native PHP then native PHP is faster. I'm happy to provide some timing if you like (I don't want to be responsible for unnecessary complexity.)

Also; would it be possible to pass a random max int instead of a bit length?

In my tests max int must be a base 2 number to work. Please don't take my word for it -- I want to be wrong. So if you want to do some of your own tests, that would be great.

When researching this, I did find a similar strategy that uses a coprime in place of prime and modulus in place of bitwise-and. That method seemed to allow any integer to be used as a max. It is described at A practical use of multiplicative inverses

@jenssegers
Copy link
Owner

@courtney-miles I started an experimental branch to try and simplify this a bit with the option of passing a max integer instead of a bit length: https://github.com/jenssegers/optimus/tree/experimental

@courtney-miles
Copy link
Contributor Author

courtney-miles commented Mar 9, 2018

hey @jenssegers , Sorry I missed your comment about the experimental branch.

Did you have any luck accommodating a max int instead of a bit length? Did you find that the resulting encoded figure would correctly decode when max int is not a base 2 number?

Is there anything I can do to help?

@jenssegers
Copy link
Owner

@courtney-miles Couldn't get it to work yet :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants