davidcole1340 / ext-php-rs

Bindings for the Zend API to build PHP extensions natively in Rust.
Apache License 2.0
572 stars 59 forks source link

feat(bindings): add bindings for cached function calls #315

Open Xenira opened 3 months ago

Xenira commented 3 months ago

For my extension I need to perform a lot of callbacks.

Using fci (fcall_info) and fcc (fcall_info_cache) yielded ~10% performance improvement.

If I find the time Ill port it into this lib. For now I just added the bindings to be able to use them in my code directly.

You might need to check the docrs_bindings.rs as it seems there are more changes then there should be (maybe php version?). Mainly u8 -> u16

Xenira commented 3 months ago

https://github.com/davidcole1340/ext-php-rs/actions/runs/9237212872/job/25414068937?pr=315 This a macos problem? Seems like there are some macos steps in the build.yml.

Xenira commented 3 months ago

Maybe specifying the arch helps. Just trying stuff though.

Xenira commented 3 months ago

Ummm, arch seems to be only supported from v2. Updated the action.

Xenira commented 3 months ago

Seems to still have x86_64 clang. install-llvm-action seems to be using cache. Can we try to clear the cache?

Xenira commented 3 months ago

How do we proceed with the pipeline issue?

ptondereau commented 3 months ago

Hey @Xenira Thank you for your patience, I've cleaned the cache and re-runned the CI and now, you have another error.

ptondereau commented 3 months ago

BTW, if you want to add more bindings, you need to have the most up-to-date stable version of PHP (8.3.7 for now) with debug mode (it exposes more symbols for bindgen)

Xenira commented 3 months ago

Hey @Xenira Thank you for your patience, I've cleaned the cache and re-runned the CI and now, you have another error.

Thanks! Did a quick google search and it seems to be a problem with dylib and the mac libc https://discourse.llvm.org/t/apples-libc-now-provides-std-type-descriptor-t-functionality-not-found-in-upstream-libc/73881

Ill try to set up my own pipeline to iterate faster on this.

BTW, if you want to add more bindings, you need to have the most up-to-date stable version of PHP (8.3.7 for now) with debug mode (it exposes more symbols for bindgen)

I'm on 8.3.7, not sure about debug though. Is that related to the differences in docrs_bindgen.rs? Because the changes itself worked fine and I was able to use the new bindings.

Xenira commented 1 month ago

Hey @ptondereau,

Finally got to test it.

This seems to be caused by clang / llvm 15 and 17. Supposedly it should work with 18, but there is no macos build yet.

I managed to get it to work by skipping the clang / llvm setup entirely for macos https://github.com/Xenira/ext-php-rs-pipeline-debug/actions/runs/10254367953/job/28368987959

How should we proceed with this? Easiest would be to skip the llvm setup for the macos runs.

ptondereau commented 1 month ago

Hey @ptondereau,

Finally got to test it.

This seems to be caused by clang / llvm 15 and 17. Supposedly it should work with 18, but there is no macos build yet.

I managed to get it to work by skipping the clang / llvm setup entirely for macos https://github.com/Xenira/ext-php-rs-pipeline-debug/actions/runs/10254367953/job/28368987959

How should we proceed with this? Easiest would be to skip the llvm setup for the macos runs.

Yes I've tried to get a full build on macos too but as you said, 18 is not available... I'm not a huge fan about skipping MacOS for the moment but maybe @danog has some plan?

Xenira commented 1 month ago

Wouldn't skip macos entirely. Just leaving out the setup steps makes macos build again https://github.com/Xenira/ext-php-rs-pipeline-debug/blob/0298ee6a016d1ddcb7178e904601485731746723/.github/workflows/build.yml#L64

ptondereau commented 1 month ago

Wouldn't skip macos entirely. Just leaving out the setup steps makes macos build again https://github.com/Xenira/ext-php-rs-pipeline-debug/blob/0298ee6a016d1ddcb7178e904601485731746723/.github/workflows/build.yml#L64

Ah ok! I'm ok about that, just leave a comment about the why and it should be good for me

Xenira commented 1 month ago

Well, now clippy complains all over the place. Will create a seperate mr to get pipeline fixed.