Unexpected performance improvements

[rcwsr]

Hi, I couldn't see a discussions tab so raised as an issue. Firstly thank you for this package it is a lifesaver!

I have a large suite of 6858 phpunit integration tests. I am running the suite twice in CI jobs, one job with the mcrypt extension and the other job with this polyfill. The suite passes fine in both cases but I am seeing an unexpected performance increase for the suite with polyfill. Both jobs are run using PHP 8.1.

1. with mcrypt compat polyfill: Time: 06:45.218, Memory: 1.22 GB
2. with mcrypt extension: Time: 10:47.279, Memory: 2.78 GB
3. a colleague's branch that uses extension (control): Time: 11:15.692, Memory: 2.74 GB

Could this be due to better garbage collection with the pure PHP implementation vs the very old c extension?

For completeness but I don't think it's relevant, I have a decorator class that wraps the mcrypt_ functions that I use, calling the phpseclib_ function directly according to an environment variable.

[terrafrost]

I don't know what job # 3 is all about but for memory utilization differences between # 1 and # 2... do you have OpenSSL installed? And how are you using mcrypt / mcrypt_compat? When OpenSSL is installed mcrypt_compat (vis-a-vis phpseclib) will use it if it can but even then it depends. Like sometimes it'll kinda half use it. Quoting the comments for the openssl_ctr_process method:

     * PHP's OpenSSL bindings do not operate in continuous mode so we'll wrap around it. Since the keystream
     * for CTR is the same for both encrypting and decrypting this function is re-used by both Crypt_Base::encrypt()
     * and Crypt_Base::decrypt(). Also, OpenSSL doesn't implement CTR for all of it's symmetric ciphers so this
     * function will emulate CTR with ECB when necessary.

[rcwsr]

We discussed privately via email particulars about the encryption methods used 👆 .

I've been having some mixed results testing this further. On my local machine I see similar results to the CI jobs. However, if i choose a test class that performed well within that suite with mcrypt_compat disabled, and run it on its own, it performs worse with mcrypt_compat than with with just the extension, which is contrary to how it behaves as a whole suite.

I've since created a PHPUnit test to purely test encrypting and decrypting a string with varying number of iterations, so that I can profile with xdebug.

I've noticed a bit of a bottleneck with phpseclib, and this may be due to the way I'm using mcrypt_compat while having the mcrypt extension enabled. Despite having the polyfill installed and enabled, phpseclib calls mcrypt_list_algorithms via \phpseclib\Crypt\Base::isValidEngine each the app calls a phpseclib_mcrypt_* function.

This is because I have a wrapper class that explicitely calls phpseclib_mcrypt_* functions, but because the extension is loaded, the polyfill obviously can't overload the extension's loaded functions, meaning \phpseclib\Crypt\Base::isValidEngine will call extension's mcrypt_list_algorithms rather than phpseclib_mcrypt_list_algorithms.

For whatever reason, the extension's mcrypt_list_algorithms() calls is much slower than phpseclib_mcrypt_list_algorithms().

However, the check in \phpseclib\Crypt\Base::isValidEngine tests if extension_loaded('mcrypt') before calling mcrypt_list_algorithms so presumably when it's not loaded, neither version of mcrypt_list_algorithms will be called anyway.

If force use of the phpseclib function in Base::isValidEngine I get these profiles (showing time), notice how

phpseclib_mcrypt_list_algorithms():

mcrypt_list_algorithms():

Not really sure how this can be applied to my original "issue" where I am seeing a faster and more memory efficient test suite using mcrypt_compat, but thought I'd make a note of my findings so far.

[rcwsr]

Some further testing with aforementioned speed test, which just encrypts and decrypts a string an increasing amount of times. I created a new image without the mcrypt extension installed:

data sets:

[
    [1],
    [1],
    [1000],
    [10000],
];

mcrypt extension enabled, polyfill enabled: 19s 338ms

mcrypt extension enabled, polyfill disabled: 1s 758ms

mcrypt extension disabled, polyfill enabled: 1s 259ms

[rcwsr]

I've just realised we're using an older version (v1) of this polyfill which uses an older version of phpseclib. v3 of phpseclib does not make this call to mcrypt_list_algorithms(). Unfortunately I've got a bit of a dependency issue preventing me from updating to the v3 of phpseclib right now.

In the meantime I am using https://github.com/asmblah/php-code-shift and https://github.com/nytris/nytris to monkey patch Base.php's use of mcrypt_list_algorithms():

My Nytris package does this, calling phpseclib_mcrypt_list_algorithms() in place of mcrypt_list_algorithms() in Base.php

    public static function install(PackageContextInterface $packageContext, PackageInterface $package): void
    {
        self::$codeShift = new CodeShift();
        self::$codeShift->shift(
            new FunctionHookShiftSpec(
                'mcrypt_list_algorithms',
                fn(callable $original) => static fn($libDir = '') => phpseclib_mcrypt_list_algorithms($libDir)
            ),
            new FileFilter('**/vendor/phpseclib/phpseclib/phpseclib/Crypt/Base.php')
        );

        self::$installed = true;
    }

[rcwsr]

I've created this Nytris/Code-shift patch to fix this issue, in the unfortunate and unlikely event that anyone else is in the same situation as me, with both the extension and polyfill installed, AND on an older version of phpseclib 😆

https://github.com/rcwsr/mcrypt-compat-speed-patch

[terrafrost]

Nice sleuthing! I'll try to take a look in the next few days, time permitting!