Uh oh!
There was an error while loading. Please reload this page.
POC: Add ext/url - #11315
Conversation
There was a problem hiding this comment.
I only removed this just to be able to compare its real performance
e67f6bb to
9365001CompareUh oh!
There was an error while loading. Please reload this page.
anonrig
commented
Jun 12, 2023
Hello! I'm one of the authors of Ada URL. How was your local benchmarks? I'd like to help and understand why this PR was closed. |
NattyNarwhal
commented
Jun 19, 2023
I wonder if curl's urlapi might be useful, since it'd be consistent with ext/curl. |
kocsismate
commented
Jun 20, 2023
To be honest, I don't really like it. But I'd only start thinking about the API if I manage to sort the rest of the problem out, specifically the issue with the C++ 17 dependency. |
iluuu1994
commented
Jun 21, 2023
Note, adding support for URL parsing from curl was rejected a year ago. https://wiki.php.net/rfc/curl-url-api I'm also not sure if making curl a required dependency is going to fly. Unless I'm misunderstanding what you're saying. |
devnexen
commented
Jun 21, 2023
Are you aiming PHP 9 or earlier ? |
kocsismate
commented
Jun 22, 2023
Possibly earlier, but I'm not sure how we can resolve the C++ dependency issue. I mean, I don't see much sense to add Ada as an optional extension, as URL parsing is pretty much a core thing.
Me neither, although I'm not sure what you mean by ditching it? I don't use string_view. But @anonrig may be able to answer it. :) |
Yes true you do not use, but ada itself use typical useful C++17 features (e.g. std::optional). |
kocsismate
commented
Jul 18, 2023
After a discussion with the PHP Foundation members, it turned out that we cannot yet rely on C++, not to mention C++ 17 indeed in case of mandatory extensions. That's why I won't continue to work on ext/url anymore since I believe such core features like URL parsing should always be available, and shouldn't be in optional extensions. That's why I'm closing this PR for now. Anyone who feels like continuing the effort should feel free to take it over. |
NattyNarwhal
commented
Jul 18, 2023
Isn't the ICU extension C++ out of necessity? A more conservative dialect, but still C++. |
anonrig
commented
Jul 18, 2023
Ada doesn't depend on ICU |
NattyNarwhal
commented
Jul 18, 2023
I was talking about |
@NattyNarwhal Yes, but ext/intl is an optional extension, meaning you can still build PHP without intl and without C++. |
This PR is purely a POC which adds support for a new extension called url building upon https://github.com/ada-url/ada. The 2 tests serve as a very simple benchmark.