Skip to content

Implement slow GD test helpers in C #17305

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

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 30, 2024

There are two test helpers which need to traverse all pixels of the given images, what is obviously rather slow in PHP. To avoid that performance penalty, we implement the functions in C and make them available as internal PHP functions, but only if GD_TEST_HELPERS is defined when building. We still keep the PHP implementations as fall- back in case someone wants to run the tests without the internal functions being defined.

We also improve the userland functions by traversing the pixels in Z order what is more cache efficient for the libgd implementation.


TODO:

  • use internal functions for all CI jobs?
  • choose better names (maybe prefix with gd_test_

There are two test helpers which need to traverse all pixels of the
given images, what is obviously rather slow in PHP.  To avoid that
performance penalty, we implement the functions in C and make them
available as internal PHP functions, but only if `GD_TEST_HELPERS` is
defined when building.  We still keep the PHP implementations as fall-
back in case someone wants to run the tests without the internal
functions being defined.

We also improve the userland functions by traversing the pixels in Z
order what is more cache efficient for the libgd implementation.
@cmb69
Copy link
Member Author

cmb69 commented Dec 30, 2024

No, this doesn't make sense. At best, we can save a couple of seconds for "normal" test runs (assuming we do some performance optimization on the userland test helpers; I'll follow up on that soon), and that doesn't outweigh the cost of having to maintain two sets of test helpers.

@cmb69 cmb69 closed this Dec 30, 2024
@cmb69 cmb69 deleted the cmb/gd-test-helpers branch December 30, 2024 23:03
@thg2k
Copy link
Contributor

thg2k commented Dec 30, 2024

Maybe #14914 would be helpful. I'd be happy to rework it.

@cmb69
Copy link
Member Author

cmb69 commented Dec 31, 2024

@thg2k, possibly. However, it is not clear what exactly we should expose to userland (discussion apparently went nowhere), and it is indeed hard to come up with a general solution. I'll try to have a closer look at #14914 and #14877 and the related discussions (https://externals.io/message/124306 and https://externals.io/message/124383, respectively).

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