[2.0] Consider abandoning the polyfill #92
Comments
|
One thing to note is that the polyfill also acts as an autocomplete source. If we decide to abandon the polyfill in 2.0, we would need to keep the stubs anyway. Something we could do is maintain a composer package that contains the tests and the stubs, and requires the extension as |
|
Without a polyfill we (amphp) will never use |
|
@kelunik but you're taking two steps back by using the polyfill at all. Anyone without knowledge of the extension using amphp will suffer serious performance and memory issues. Edit: noticed that php-ds is not actually a dependent of amphp? |
|
Right, we currently don't use it, but if we would, we'd want it to be runnable by default without any additional extensions. It's currently not worth for the few cases we have. |
|
With first installation attempt fail, I've installed the extension without any issue on a new php build. I'm new to php-ds and still doing testing, I don't see any reason not to abandoned it if nobody is using it. |
|
@TheCelavi @jkuchar @dantudor @krakjoe @nikic in light of this issue and #8 (Pure PHP implementation), I would like to ask if anyone has any strong opinions here. In my opinion: Reasons to abandon:
Reason to maintain:
|
|
It was big thing when I was considering if we are going to use phpds interface. For example when there will be new version of php and this extension will not be available (e.g. not maintained anymore) I will still be able to use the app thanks to the polyfill or even fix someting there. I will not be able to fix C source. Polyfill makes me feel much more safe that my app will not be locked on old php version where phpds used to be supported.
I use polyfill all the time because I do not want to mess with the compilation process on Windows. I do not care that it is slow. It works.
It makes sense to me to NOT have polyfill if phpds makes it into php bundled extensions. (e.g. just enable it in php.ini and go; or just install package and go on linux)
Honza Kuchař
23. 7. 2017 v 3:13, Rudi Theunissen <[email protected]>:
… @TheCelavi @jkuchar @dantudor @krakjoe @nikic in light of this issue and #8 (Pure PHP implementation), I would like to ask if anyone has any strong opinions here.
In my opinion:
Reasons to abandon:
No chance that the library decreases performance (either extension or not at all).
No need to maintain the second implementation - can focus on the extension.
There isn't a practical reason why anyone would be using the polyfill, eg. if you're in an environment where you don't have access to install the extension, you shouldn't be using the library at all.
Reason to maintain:
Improves adoption, can play around without installed the extension.
Allows code to run in an environment where the extension isn't installed.
Doubles as autocomplete stubs.
Most of it is already implemented.
We'll have to maintain it for 1.0 anyway.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@jkuchar I think what dawned on me is that you would be better off not upgrading PHP or not using the extension than if you were using the polyfill.
You shouldn't need to at all, there are |
|
I didn't know it made it into PHP 7.1 default bundle. Now it feels like something stable.
The mission of polyfill - to overcome the fear of getting dependent on new extension whose development can be abandoned - is completed.
25. 7. 2017 v 2:53, Rudi Theunissen <[email protected]>:
… @jkuchar I think what dawned on me is that you're better of either not upgrading PHP or not using the extension than you are using the polyfill.
I do not want to mess with the compilation process on Windows
You shouldn't need to at all, there are .dll's in the releases.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Not that I'm aware of, but it's compiled and available on PECL.
The mission of the polyfill is:
I'm starting to feel like maybe failing hard when the extension is not available is a good thing. Because you'd be well worse off then than if you were using arrays. I'm not convinced either way yet, so this is still just a consideration. 🤔 |
|
I agree with abandoning the polyfill. If the performance is not there then it's not worth having (it defeats the purpose of using ds). Maintaining the polyfill takes effort away from the extension. It shouldn't be that hard to maintain stubs for autocomplete. |
|
That's what I'm leaning towards at the moment. I fail to see what value it has going forward. Either use the extension or don't use the library at all. |
|
How bad is the performance of the polyfill? |
|
It's not horrendous. The only operation that makes me nervous is put and get on Map being O(n). Haven't done any benchmarks. |
|
The polyfill test suite runs on my mac: extension: |
OMG. That's really bad. If that's the state of the polyfill implementation, I would definitely abandon it. On the other hand, with that issue resolved performance might be acceptable. |
|
I don't know if it's a solvable problem, unless we build a hashtable using an array, but the memory usage would be huge if we do that. Strong types on the keys and objects as keys makes it difficult to use an array as the internal store because we can't index. On top of that we still have to maintain insertion order. |
|
@rtheunissen The current definition of the Hashable interface is what makes this hard. If Hashable::hash() would be required to uniquely identify an object (of the given class) it should be easy to implement something relatively efficient. |
|
@nikic that would make it easy yes, but that's not realistic here. We can't use |
|
@rtheunissen Given how people would implement this function in userland, I don't think it's unrealistic. As we don't expose hashing primitives (for strings in particular), the most obvious ways of implementing hash() are also going to be collision-free. |
|
For example, a database entity's primary key? I can see how the common implementation would double as an identity function. But I don't like the idea of changing the definition simply for the sake of accomodating the polyfill. There's also the case of using an array as a key.. where the hash is now the length of the array, and get/put is O(k) because we have to compare the elements of the array. Intuitively I can't get my head around how we can do this efficiently in userland without a proper native array type that we can use to build a hashtable. |
|
I wouldn't worry about performance of array keys, that's a very unusual use-case. You can't do this really efficiently in userland, but implementing a userland HT should also not be much less memory efficient than what you have now (due to the general object overhead, adding two more fields on top of Pair is not going to double the memory usage). But then again, implement anything here is going to take time and I can see how that time might be better spent on something else... |
|
Yeah I just don't see the point of going out of our way here. |
That only works if the user of the library is also controlling the system where the app is to be deployed. |
|
@kelunik which is why I'm saying if you don't control the system, don't use the library. In saying that, if it's a dependency of another dependency, you're in trouble. But how is that any different to extensions like mongo or pthreads? My concern is that if you don't have control of the system now, you probably won't ever have, and you're then stuck with slow code under the hood. Could argue that it's then insignificant because you won't be processing a lot if you're not controlling your system anyway. |
Right, on those systems I / we don't care about performance, but pure portability and runability.
We abstract those where possible, e.g. using multiple processes instead of threads where they're not available in https://github.com/amphp/parallel. If you require mongo to run, you can also require the extension, because the user has to be in control then anyway to run mongo. But that's nothing desirable for generic data structures. It definitely depends on the use case, but for some cases it's just easier using arrays or Spl then, even if they're worse implementation wise. |
In that case I would recommend making 2.0 of this extension with the low-level structures and build a composer package on top of them as you said. Then have the community use it like that and focus on further improving and stabilizing the low-level structures before making a RFC to include them in PHP core. I agree that moving to core soon is not necessarily a good idea, but it would be nice to prepare DS 2.0 with that in mind as eventual goal. Having to focus on and stabilize just a few low level structures should make this goal easier. Yes, high level structures would lose some performance that way but in my opinion it's reasonable in this case. High level structures can be implemented in C later on as well. |
I'm not too phased about this to be honest. If it stabilizes well and adoption is good, we can consider a proposal but at this point the benefit isn't obvious to be. |
|
Devil's Advocate here. You folks seem quite preoccupied with the performance of the polyfill as compared to the extension, and there's no doubt that the extension is superior in virtually every respect. However, one thing that doesn't seem to have been considered within the context of this discussion is the performance of the equivalent data structures that would be written by bozos like me who aren't as intimately familiar with the minutiae of PHP internals or even the underlying concepts themselves. Given enough rope to hang myself I might just base everything on a linked list and call it a day. Taken on its own as a userland implementation of the data structures it is some very fine code that I would recommend people use. Within this context I think that the polyfill should be considered a gateway to the extension, which is a point that I've seen raised several times already. I would suggest addressing the performance concerns via documentation and perhaps an informative message in a Composer |
|
I'm not sure what the current state of development is regarding the entire discussion in here, but I landed here wanting to use the data structures in my microframework. My decision to use it is heavily based on the polyfill, because I would want the microframework to work for people who can't/don't want to install the extension. For people who want the performance gain, they would go the extra mile. If you're looking for greater adaptation, you have got to build for the bigger audience, and the bigger audience of PHP doesn't care for extensions all too much. |
|
We are actively planning ext-ds 2.0 on the 2.0 branch, and at this stage I do not foresee a polyfill implementation that mirrors the extension. I believe that this extension will only be successful if it is required, otherwise 3/5 projects will never actually install it and never truly depend on it or get any value from it. If you want to use them in your microframework, you should consider a hard dependency on the extension, or internal implementations of the interfaces. What we could also do.. is provide a standard userland implementation of the interfaces, but have the classes not aliased. Eg. I really, really want to avoid this though. |
|
Totally understand. In that case, you should really stick to your priorities. Interfaces with a basic implementation sounds like a nice idea too, it might work to add a fallback when the extension doesn't exist. When do you plan to release version 2? Do you have a date yet? And do you have anything new planned? Would love to hear about it. |
Hopefully by the end of the year. If I could work on this full-time, maybe 3 months from now. I'm rewriting everything so don't expect much backward-compatibility. ^^ FWIW check out https://github.com/php-ds/ext-ds/blob/2.0/DRAFT.php |
|
@rtheunissen Does it make sense to have a separate extension name, so both versions could be installed at the same time? |
|
@kelunik I am open to that, but I haven't considered it. I think just a new major version is okay if we can somehow preserve the documentation from v1. |
|
@rtheunissen Allowing both versions being installed at the same time would allow for a smoother transition, because you can migrate things partially instead of all or nothing. |
|
Wouldn't that just cause more confusion? The namespace would have to change as well. I think either using one or the other is the best way forward. Following the migration guide should be easy enough to not require a temporary mixed state. |
|
Depends on how much changes, if you have libraries and own code depending on |
|
Supporting two versions of the same library at the same time is madness. 😂 |
|
The existence of polyfill was the reason I even considered adopting I'm ok if there's worse performance using the polyfill, if users (of my software) have the performance as an issue, they can install the extension. and I'm perfectly fine with that. Thank you for maintaining both versions for now! Perhaps the end goal should be to get support classes to php-core, like (sorry I did not read the full thread here if this POV was already covered). |
Look at my brand new PR at php-ds/polyfill#51: It's a PhpStorm stub. You could make that ENTIRE source code of record for the entire php-ds/php-ds composer package, completely abandon the php-ds/polyfill and STILL have 100% code completion + documentation. In fact, once the PhpStorm stub is officially accepted by PhpStorm (Open a PR over at https://github.com/JetBrains/phpstorm-stubs), then that's EXACTLY what I recommend doing, professionally, if performance really is terrible and a design problem. Because there are over 250,000 installs reported by packagist, I'd recommend deprecating There is literally zero warning about performance, memory, etc. on the polyfill README. I had no idea it was "that bad" until I stumbled upon this thread. I also highly second everything @yybalam said in his php-ds/polyfill apology. |
|
I haven't seen this mentioned yet, but perhaps in PHP 7.4 it would become a possibility to use the FFI to implement this extension in PHP instead? I realize the first version of the FFI does not have the performance of an actual extension, but IIRC there are plans to improve this (i.e. JIT in PHP 8). |
|
FFI will be slower then native extension, but still with the same big-O complexity (just with added constant penalty for each function call, which is more then acceptable). |
|
link for lazy ones about ffi that got merged: https://wiki.php.net/rfc/ffi |
|
@tominventisbe @jkuchar for a project like this (data structures being a special case), performance and low-level functionality is worth more than the benefit of using the FFI / implementing it in PHP. I don't know that for sure, but I would probably vote to keep the C implementation as the primary, and potentially implement it with FFI as well as an alternative option. |
|
if you want wider adaptation then FFI looks like a really good option. It may be slow when it's introduced, but surely it will improve in newer versions |
|
I don't understand how to use FFI to "polyfill" this. If you have the extension, then use it. If you don't have it, how are you going to call into it via FFI? Am I missing something? |
I disagree with this. No one else is calling for polyfill implementations of other extensions. If a library/extension provides enough value for enough people, it will be adopted by the community. Good examples are PDO, xdebug, gd/imagick, etc. We would be spending a lot of time implementing and supporting an inferior version of something simply to avoid the need to install and enable an extension, which in my opinion indicates that the problem to solve is part of PHP's dependency management infrastructure, rather than extensions themselves. If a polyfill must exist, it won't concern itself with FFI. |
I read it as a rewrite/alternative version of the main implementation, but instead of an extension it's written in PHP using native C bits and pieces? Anyone is welcome to attempt that, but it will not be an effort within the php-ds repo, at least from my side. |
|
FFI is just a bridge to create bindings for shared libraries ( |
|
Yes, unfortunately, as I see it (though I may be missing something): if you also want to keep the extension around, you'd still be stuck maintaining two code bases as the FFI allows you to write extensions in PHP by binding to e.g. C libraries from PHP, as already mentioned by @glensc. Since it's mostly real PHP code and some PHP wrapping/mapping the C functions, it's not native (e.g. C) code like a native extension would be, so you'd still need to maintain both. You could use the FFI to load in a new php-ds C library instead (which would then also be used by the extension), but this then poses the problem of getting that new C library on your (shared) hosting. This could be done in some cases via static libraries - I believe there are some examples of this, such as I believe the most ideal course of action would be to "simply" (try and get) this extension merged in PHP itself as core extension. I think there is room for a good (better) set of container and data structures in PHP, though I'm not a core developer, who may think differently. There may also be requirements, such as the extension having a certain amount of popularity if it is useful enough, such as mentioned by @rtheunissen. I must admit I'm afraid that this is not one of those types of extensions that is going to pull that amount of weight as it's not like databases where you may need to communicate with something a customer demands, and thus pretty much need the extension without question and get it installed. This library is something that is usually seen as optional as there are some alternatives in pure PHP on Packagist (though arguably not as good and extensive, or we wouldn't be having this discussion 😄), but there is little hard material to convince people to have such an extension installed on their servers - if that is at all possible and they are not on shared hosting - unless they require the performance, which is also a minority, I believe. If they don't require the performance, they are probably going to look for something not requiring any installation, especially since a custom extension also poses an additional security vector. |
|
If the polyfill is abandoned, so too goes the adoption. It's literally the only reason I'll risk distributing code w/ php-ds. |
This makes me sad, and I'm not sure if I agree, but I am open to the idea of a polyfill as long as it is strongly recommended that projects install the extension if it is at all possible to do so. I don't like the idea of potentially slowing down what could be most projects if the aversion to install an extension is shared by a majority. I'm just curious... if you are writing something that you would like to distribute (library, etc), and you'd like to use php-ds, why not require php-ds as a dependency? If a user can't install it, they also can't use your library. The problem to solve here is why they can't install it (or don't want to), and I worry that maintaining a polyfill encourages that aversion, and discourages the innovation we need around extension dependencies. Classic pragmatism vs. idealism situation here. Do we bite the bullet and maintain the polyfill for the sake of practicality, or do we insist that others should get with the times or be left behind? Looking at 2.0, the API will almost definitely be trimmed way down, which would make the polyfill a lot easier to manage. If I had to vote on this today, I think I would vote to keep it because there is no clear consensus to abandon it. But if I ever hear of someone not installing the extension by choice... 👮♂️ So until further notice, the decision here is to keep it. 🎉 |
There are a few reasons why I believe we should consider abandoning the polyfill:
==to behave like it does for arrays.The text was updated successfully, but these errors were encountered: