Re: [PHP-DEV] State of Generics and Collections
Hi Larry, On Tue, Aug 20, 2024 at 3:32 AM Larry Garfield wrote: > > In fact, generic traits (essentially statically replacing the generic > > arguments at link-time) would be an useful feature which would remain > > useful even if we had fully reified generics. > > I recognize that some functionality will need support of internal > > zend_object_handlers. But that's not a blocker, we might provide some > > default internal traits with PHP, enabling the internal class handlers. > > So to summarize, I would not continue on that path, but really invest > > into monomorphizable generic traits instead. > > Interesting. I have no idea why Arnaud has mainly been investigating reified > generics rather than monomorphized, but a monomorphized trait has potential, > I suppose. That naturally leads to the question of whether monomorphized > interfaces would be possible, and I have no idea there. (I still hold out > hope that Levi will take another swing at interface-default-methods.) > > Though this still wouldn't be a path to full generics, as you couldn't > declare the inner type of an object at creation time, only code time. Still, > it sounds like an area worth considering. Monomorphization as a solution to generic classes has a memory usage issue (it requires duplicating the class entry, methods, props, and also opcodes if method bodies can reference type parameters), and does not solve all the complexity: https://github.com/PHPGenerics/php-generics-rfc/issues/44. This would be less a problem for traits, as there is already some amount of duplication. Best Regards, Arnaud
Re: [PHP-DEV] State of Generics and Collections
Hi Mike, On Tue, Aug 20, 2024 at 2:45 AM Mike Schinkel wrote: > It seems Java-style Generics are viewed as the proper archetype for Generics > in PHP? I would challenge the wisdom of taking that road considering how > different the compilers and runtimes are between the Java and PHP. PHP > should seek out solutions that are a perfect fit for its nature and not > pursue parity with Java. > As PHP is primarily a web development language — vs. a systems language like > C or Rust, or an enterprise application language like Java or C# — reducing > code complexity for reading and understanding is a very important attribute > of the language. > PHP is also a unique language and novel solutions benefit a unique language. > PHP should pursue solutions that result in less complex code even if not > found in other languages. Your collections idea is novel — which is great — > but there are probably even more novel solutions to address other needs vs. > going full-on with Java-style generics. > Consider if adding type aliases; or augmenting, enhancing, or even merging > classes, interfaces, and/or traits to address the needs Java-style generics > would otherwise provide. I would work on some examples but I think you are > more likely to adopt the features you come up with on your own. Part of the appeal for Java/C#/Kotlin-like generics is that they are well understood and their usefulness is not to be proven. Also they fit well with the object-oriented aspect of the language, and many PHP projects already use them via PHPStan/Psalm. More experimental alternatives would be more risky. I would be interested to see suggestions or examples, however. > As for type-erasure, I am on the fence, but I find the proposed "how" > problematic. > I can see wanting some code to be type-checked and other code not, but I > think more often developers would want code type-checked during development > and testing but not for staging or production. And if the switch for that > behavior is in every file that means modifying every file during deployment. > IMO that is just a non-starter. The reason for this "how" is that type checking is also coercing, so disabling it "from the outside" may break a program that's not designed for that. That's why this is something that should be enabled on a per-file basis, and can probably not be switched on/off depending on the environment. > P.S. Also consider offering the ability for a function or class method to > "type" a parameter or variable based on an interface and then allow values > that satisfy that interface structurally[1] but not necessarily require the > class to explicitly implement the interface. Unfortunately, I believe that structural types would be very expensive to implement at runtime. Static analysers could support this, however (PHPStan/Psalm support some structural types already). Best Regards, Arnaud
Re: [PHP-DEV] State of Generics and Collections
Hi Bob,
On Tue, Aug 20, 2024 at 12:18 AM Bob Weinand wrote:
> The fluid Arrays section says "A PoC has been implemented, but the
> performance impact is still uncertain". Where may I find that PoC for my
> curiosity? I'm imagining the implementation of the array types as a counted
> collection of types of the entries. But without the PoC I may only guess.
I may publish the PoC at some point, but in the meantime here is a
short description of how it's implemented:
- The zend_array has a zend_type member representing the type of its elements
- Everytime we add or update a member, we union its type with the
array type. For simple types it's just a |= operation. For arrays with
a single class it's also simple. For complex types it's more expensive
currently, but it may be possible to cache transitions to make this
cheaper.
- Updating the array type on deletes requires to either maintain a
counter of every type, or to re-compute the type entirely everytime.
Both are probably too expensive. Instead, we don't update the type on
deletes, but we re-compute the type entirely when a type check fails.
This is based on two hypotheses: 1. A delete rarely changes an array's
type in practice, and 2. Type checks rarely fail
- References are treated as mixed, so adding a reference to an array
or taking a reference to an element changes its type to mixed. Passing
an array to a more specific array will cause a
re-compute, which also de-refs every reference.
- Updating a nested element requires updating the type of every parent
> It also says "Another issue is that [...] typed properties may not be
> possible.". Why would that be the case? Essentially a typed property would
> just be a static array, which you describe in the section right below.
It becomes complicated when arrays contain references or nested
arrays. Type constraints must be propagated to nested arrays, but also
removed when an array is not reachable via a typed property anymore.
E.g.
class C {
public array> $prop;
}
$a = &$c->prop[0];
$a[] = 'string'; // must be an error
unset($c->prop[0]);
$a[] = 'string'; // must be accepted
$b = &$c->prop[1];
$b[] = 'string'; // must be an error
$c->prop = [];
$a[] = 'string'; // must be accepted
I don't remember all the possible cases, but I didn't find a way to
support this that didn't involve recursively scanning an array at some
point. IIRC, without references it's less of an issue, so a possible
way forward would be to forbid references to members of typed
properties. Unfortunately this breaks pass-by-reference, e.g.
`sort($c->prop)`. out/inout parameters may be part of a solution, but
with more array separations than pass-by-ref.
Best Regards,
Arnaud
Re: [PHP-DEV] State of Generics and Collections
Hi Rob,
On Mon, Aug 19, 2024 at 7:51 PM Rob Landers wrote:
> > Invariance would make arrays very difficult to adopt, as a library can not
> > start type hinting generic arrays without breaking user code, and users can
> > not pass generic arrays to libraries until they start using generic arrays
> > type declarations.
>
> This seems like a strawman argument, to a degree. In other words, it seems
> like you could combine static arrays and fluid arrays to accomplish what you
> are seeking to do. In other words, use static arrays but allow casting to
> treat it as "fluid."
>
> In other words, simply cast to get your example to compile:
>
> function f(array $a) {}
> function g(array $a) {}
>
> $a = (array) [1]; // array unless cast
>
> f($a); // ok
> g((array)$a); // ok
>
> And the other way:
>
> function f(array $a) {}
> function g(array $a) {}
>
> $a = [1];
>
> f((array)$a); // ok, type check done during cast
> g($a); // ok
There is potential for breaking changes in both of your examples:
If f() is a library function that used to be declared as `f(array
$a)`, then changing its declaration to `f(array $a)` is a
breaking change in the Static Arrays flavour, as it would break
library users until they change their code to add casts.
Similarly, the following code would break (when calling g()) if h()
was changed to return an array:
function h(): array {}
function g(array $a);
$a = h();
g($a);
Casting would allow users to pass generic arrays to libraries that
don't support generics yet, but that's expensive as it requires a
copy.
Best Regards,
Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Tim, On Thu, Jul 25, 2024 at 4:47 PM Tim Düsterhus wrote: > I'm seeing there are some more changes and not just to the cloning > section. I've went through the entire RFC once again and here are my > (hopefully final) five remarks. They are only about textual > clarification of some behaviors, I don't have any further semantic concerns. Thank you! We have updated the RFC accordingly. > 5. In the explanation of > "ReflectionClass::markLazyObjectAsInitialized()", it says: > > > Its behavior is the same as described for Ghost Objects in the > > Initialization Sequence section, except that the initializer is not > > called. > > > > This means that calling `markLazyObjectAsInitialized()` on a lazy proxy > turns it into a regular object of the proxy class, as if > `newInstanceWithoutConstructor()` was used, right? Yes, except for the value of properties that were already initialized. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Philip, On Thu, Jul 18, 2024 at 12:19 PM Philip Hofstetter wrote: > Super minor nitpick: You have updated the prototype, but not the explanation > text which still says: > >> When initialization is required, the $initializer is called with the object >> as first parameter. The initializer should initialize the object, and must >> return null (or void). See the “Initialization Sequence” section. I've updated the text as suggested, as the Initialization Sequence section also specifies how the function should return. The Initialization Sequence section says approximately the same thing, however: "The initializer must return null or no value. If the initializer returns something other than null, a TypeError is thrown". The reason is that all function calls evaluate to a value, including when calling a void function, so void functions actually return null. Unfortunately we have no good way to enforce a void return, unless we check the declared return type of the function, but it would not work for all kinds of callables. Documenting the signature as returning void is fine, though. Note that the return value is not important currently: We just check that it's null so that we reserve the possibility to give the return value a meaning in the future. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Tim, On Sun, Jun 30, 2024 at 3:54 PM Tim Düsterhus wrote: > On 6/27/24 16:27, Arnaud Le Blanc wrote: > >> * flags should be a `list` instead. A bitmask for > >> a new API feels unsafe and anachronistic, given the tiny performance hit. > >> > > > > Unfortunately this leads to a 30% slowdown in newLazyGhost() when switching > > to an array of enums, in a micro benchmark. I'm not sure how this would > > impact a real application, but given this is a performance critical > > I'm curious, how did the implementation look like? Is there a proof of > concept commit or patch available somewhere? As the author of the first > internal enum (Random\IntervalBoundary) I had the pleasure of finding > out that there was no trivial way to efficiently match the various enum > cases. See the PR review here: > https://github.com/php/php-src/pull/9679#discussion_r1045943444 I've benchmarked this implementation: https://github.com/arnaud-lb/php-src/commit/f5f87d8a7abeba2f406407606949e5c6e512baab. Using a backed enum to have a more direct way to map enum cases to integers didn't make a significant difference. Here is the benchmark: https://gist.github.com/arnaud-lb/76f77b5d7409a9e4deea995c179c6e96. Caching the options array between calls had a less dramatic slowdown (around 10%): https://gist.github.com/arnaud-lb/87e7f58cc11463dd3aa098218eb95a90. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Marco, On Thu, Jun 27, 2024 at 12:32 PM Marco Pivetta wrote: > > Hey Arnaud, > > On Wed, 26 Jun 2024 at 21:06, Arnaud Le Blanc wrote: >> >> The proposed implementation is adding very little complexity as it's not >> adding any special case outside of object handlers (except in json_encode() >> and serialize() because these functions trade abstractions for speed). >> Furthermore all operations that may trigger an object initialization are >> already effectful, due to magic methods or hooks (so we are not making pure >> operations effectful). This means that we do not have to worry about lazy >> objects or to be aware of them anywhere in the code base, outside of object >> handlers. >> >> To give you an idea, it's implemented by hooking into the code path that >> handles accesses to undefined properties. This code path may call __get or >> __set methods if any, or trigger errors, and with this proposal, may trigger >> the initialization. Userland implementations achieve this functionality in a >> very similar way (with unset() and a generated sub-class with magic >> methods), but they have considerably more edge cases to handle due to being >> at a different abstraction level. > > > Assuming this won't pass a vote (I hope it does, but I want to be > optimistic): is this something that could be implemented in an extension, or > is it only feasible in core? An extension could achieve similar behavior by decorating the default object handlers. However, it may have to re-implement a significant part of the object handlers logic, so that initialization is triggered at the right time. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Rob,
On Wed, Jun 26, 2024 at 11:09 PM Rob Landers wrote:
> Can you add to the RFC how to proxy final classes as well? This is mentioned
> (unless I misunderstood) but in the proxy example it shows the proxy class
> extending the proxied class (which I think is an error if the base class is
> final). How would this work? Or would it need to implement a shared interface
> (this is totally fine IMHO)?
The example you are referring to in the "About Proxies" section is a
digression about how the lazy-loading inheritance-proxy pattern could
be achieved on top of the lazy-loading state-proxy pattern implemented
by this RFC, but it doesn't represent the main use-case.
To proxy a final class with this RFC, you can simply call the
newLazyProxy method:
final class MyClass {
public $a;
}
$reflector = new ReflectionClass(MyClass::class);
$obj = $reflector->newLazyProxy(function () {
return new MyClass();
});
Best Regards,
Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Marco,
Thank you for the very detailed feedback. Please find my answers below.
Nicolas will follow-up on some points.
On Mon, Jun 24, 2024 at 1:03 AM Marco Pivetta wrote:
> * lazy proxies should probably explore replacing object identity further
>
We have thought about a few approaches for that, but none is really
concluding:
Fusioning the identity of the proxy and the real instance, such that both
objects are considered to have the same identity with regard to
spl_object_id(), SplObjectStorage, WeakMap, and strict equality after
initialization will lead to weird effects.
For example, we don't know what the behavior of this should be:
```
$reflector = new ReflectionClass(MyClass::class);
$realInstance = new MyClass();
$proxy = $reflector->newLazyProxy(function () use ($realInstance) {
return $realInstance;
});
$store = new SplObjectStorage();
$store[$realInstance] = 1;
$store[$proxy] = 2;
$reflector->initialize($proxy);
var_dump(count($store)); // 1 or 2 ?
```
A second approach, suggested by Benjamin, would be that properties of the
instance returned by the factory are copied to the proxy, the proxy marked
as initialized (so it's not a proxy anymore), and the instance returned by
the factory discarded. One problem of this approach is that the instance
may continue to exist independently of the proxy if it's referenced
somewhere. So, proper usage of the lazy proxy API would require the factory
to be aware of that, and to return an object that is not referenced
anywhere. As we implemented the proxy strategy primarily for use-cases
where we don't control the factory, this approach would not work.
A third approach would be to replace all references to the proxy by
references to the backing instance after initialization. Implementing this
approach would be prohibitively slow as it requires to scan the entire
object graph of the process.
None of these approaches are concluding unfortunately. Having two distinct
identities for the proxy and the real instance doesn't appear to be an
issue in practice, however (e.g. in Symfony).
* don't like expanding `ReflectionClass` further: `LazyGhost` and
> `LazyProxy` classes (or such) instead
>
The rationale for expanding ReflectionClass and ReflectionProperty is that
code creating lazy objects tend to also use these two classes, for
introspection or to initialize the object. Also, we feel that the new
methods are directly related to existing methods. E.g. newLazyGhost() is
just another variant of newInstance() and newInstanceWithoutConstructor(),
and setRawValueWithoutLazyInitialization() is just another variant of
setValue() and setRawValue() (added in the hooks RFC).
* `initialize()` shouldn't have a boolean behavioral switch parameter: make
> 2 methods
>
Agreed. We have updated the RFC to split the method into `initalize()` and
`markAsInitialized()`.
> * flags should be a `list` instead. A bitmask for
> a new API feels unsafe and anachronistic, given the tiny performance hit.
>
Unfortunately this leads to a 30% slowdown in newLazyGhost() when switching
to an array of enums, in a micro benchmark. I'm not sure how this would
impact a real application, but given this is a performance critical
feature, the slowdown is an issue. Besides that, no existing API in core is
using an array of enums, and we don't want to introduce this concept in
this RFC.
> From an abstraction point of view, lazy objects from this RFC are
> indistinguishable from non-lazy ones
* do the following aspects always apply? I understand they don't for lazy
> proxies, just for ghosts?
>* `spl_object_id($object) === spl_object_id($proxy)`?
>* `get_class($object) === get_class($proxy)`?
>
Good catch, this phrase is not true for proxies, with relation to the
identity of a proxy and its real instance. Apart from that, the main point
of this phrase remains: interaction with a proxy has the same behavior as
interaction with a real instance, and they can be used without knowing they
are lazy.
> Proxies: The initializer returns a new instance, and interactions with
> the proxy object are forwarded to this instance
> * another note on naming: I used "value holder" inside ProxyManager,
> because using just "proxy" as a name led to a lot of confusion. This also
> applies to me trying to distinguish proxy types inside the RFC and this
> discussion.
>
The name "Value Holder" appears to be taken for another pattern already:
https://martinfowler.com/eaaCatalog/lazyLoad.html
The exact name of the pattern used in this RFC is the "Virtual
State-Proxy", but others have suggested using just "Proxy" in the RFC. In
the API we use "LazyProxy".
> Internal objects are not supported because their state is usually not
> managed via regular properties.
>
> * sad, but understandable limitation
> * what happens if a class `extends` an internal engine class, like the
> gruesome `ArrayObject`?
>
This also applies to sub-classes of internal objects. This is specified
under ReflectionClass::n
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Gina, On Tue, Jun 25, 2024 at 5:59 PM Gina P. Banyard wrote: > The fact that an initialize() method has a $skipInitializer parameter > doesn't make a lot of sense to me. > Because at a glance, I don't see how passing true to it, and not calling > the method is different? > Calling initialize() with $skipInitializer set to true has the same effect as calling skipLazyInitialization() on all properties that are still lazy on the object. This can be used to manually initialize a lazy object afterwards, as property accesses will not trigger initialization anymore. This also ensures that the initializer function can be decref'ed. This should probably be split into two distinct methods. > Agreed > Does get_mangled_object_vars() trigger initialization or not? > No. get_mangled_object_vars() and array cast are among the few cases that do not trigger initialization. They are listed in https://wiki.php.net/rfc/lazy-objects#initialization_triggers. > This should behave like an (array) cast (and should be favoured instead of > an array cast as it was introduced for that purpose). > Exactly > How does a lazy object look like when it has been dumped? > The output of var_dump() on a lazy object is the same as on an object whose all properties have been unset() (except those initialized with setRawValueWithoutLazyInitialization() or skipLazyInitialization()). For convenience we also prefix the output with either `lazy ghost` or `lazy proxy`. I've added a var_dump section in the RFC. > > The initializer must return null or no value > *Technically* all functions in PHP return a value, which by default is > null, so this is somewhat redundant. > Agreed. I believe that formulating this like that makes it clear that any of "return null;", "return;", or implicit return, will work. Also, would this throw a TypeError if a value other than null is returned? > Agreed. Currently we throw an Error, but I will change that to TypeError. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Levi, On Wed, Jun 26, 2024 at 12:07 AM Levi Morrison wrote: > I will vote no on this one. I do not believe the internal complexity > and maintenance is worth the feature. Additionally, I do not feel like > changing the language to support this feature is a good idea; if this > were a library only thing, I would not care. > Hi Levi, The proposed implementation is adding very little complexity as it's not adding any special case outside of object handlers (except in json_encode() and serialize() because these functions trade abstractions for speed). Furthermore all operations that may trigger an object initialization are already effectful, due to magic methods or hooks (so we are not making pure operations effectful). This means that we do not have to worry about lazy objects or to be aware of them anywhere in the code base, outside of object handlers. To give you an idea, it's implemented by hooking into the code path that handles accesses to undefined properties. This code path may call __get or __set methods if any, or trigger errors, and with this proposal, may trigger the initialization. Userland implementations achieve this functionality in a very similar way (with unset() and a generated sub-class with magic methods), but they have considerably more edge cases to handle due to being at a different abstraction level. Best Regards, Arnaud
Re: [PHP-DEV] [Early Feedback] Pattern matching
On Fri, Jun 21, 2024 at 7:20 PM Robert Landers wrote: > > > I'm always surprised why arrays can't keep track of their internal > > > types. Every time an item is added to the map, just chuck in the type > > > and a count, then if it is removed, decrement the counter, and if > > > zero, remove the type. Thus checking if an array is `array` > > > should be a near O(1) operation. Memory usage might be an issue (a > > > couple bytes per type in the array), but not terrible but then > > > again, I've been digging into the type system quite a bit over the > > > last few months. > > > > And every time a modification happens, directly or indirectly, you'll > > have to modify the counts too. Given how much arrays / hash tables are > > used within the PHP codebase, this will eventually add up to a lot of > > overhead. A lot of internal functions that work with arrays will need > > to be audited and updated too. Lots of potential for introducing bugs. > > It's (unfortunately) not a matter of "just" adding some counts. > > Well, of course, nothing in software is "just" anything. Counters are not cheap as we need one slot for each type in the array so we need a dynamic buffer and an indirection, plus absolutely every mutation needs to update a counter, including writes to references. It is possible to remove the counters and to maintain an optimistic upper bound of the type (computing the type more precisely when type checking fails), but I feel this would not work well with pattern matching. Also, a few things complicate this: - Nested writes like $a[0][0][0]=1 need to backtrack to update the type of all parent arrays after the element is added/updated - Supporting references to properties whose type is a typed array, or dimensions of these properties, is very hard Fixed-type arrays may be easier to support but there are important drawbacks in usability IMHO. This does not play well with CoW semantics. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Larry,
Following your feedback we propose to amend the API as follows:
```
class ReflectionClass
{
public function newLazyProxy(callable $factory, int $options): object {}
public function newLazyGhost(callable $initializer, int $options): object {}
public function resetAsLazyProxy(object $object, callable
$factory, int $options): void {}
public function resetAsLazyGhost(object $object, callable
$initializer, int $options): void {}
public function initialize(object $object): object {}
public function isInitialized(object $object): bool {}
// existing methods
}
class ReflectionProperty
{
public function setRawValueWithoutInitialization(object $object,
mixed $value): void {}
public function skipInitialization(object $object): void {}
// existing methods
}
```
Comments / rationale:
- Adding methods on ReflectionClass instead of ReflectionObject is
better from a performance point of view, as mentioned earlier
- Keeping the word "Lazy" in method names is clearer, especially for
"newLazyProxy" as a the "Proxy" pattern has many uses-cases that are
not related to laziness. However we removed the word "Instance" to
make the names shorter.
- We have renamed "make" methods to "reset", following your feedback
about the word "make". It should better convey the behavior of these
methods, and clarify that it's modifying the object in-place as well
as resetting its state
- setRawValueWithoutInitialization() has the same behavior as
setRawValue() (from the hooks RFC), except it doesn't trigger
initialization
- Renamed $initializer to $factory for proxy methods
WDYT?
Best Regards,
Arnaud
On Sun, Jun 16, 2024 at 3:46 PM Arnaud Le Blanc wrote:
>
> On Sat, Jun 15, 2024 at 7:13 PM Larry Garfield wrote:
> > > In practice I expect there will be two kinds of ghost initializers:
> > > - Those that just call one public method of the object, such as the
> > > constructor
> > > - Those that initialize everything with ReflectionProperty::setValue()
> > > as in the Doctrine example in the "About Lazy-Loading strategies"
> > > section
> > I'm still missing an example with ::bind(). Actually, I tried to write a
> > version of what I think the intent is and couldn't figure out how. :-)
> >
> > $init = function() use ($c) {
> > $this->a = $c->get(ServiceA::class);
> > $this->b = $c->get(ServiceB::class);
> > }
> >
> > $service = new ReflectionLazyObjectFactory(Service::class, $init);
> >
> > // We need to bind $init to $service now, but we can't because $init is
> > already registered as the initializer for $service, and binding creates a
> > new closure object, not modifying the existing one. So, how does this even
> > work?
>
> Oh I see. Yes you will not be able to bind $this in a simple way here,
> but you could bind the scope. This modified example will work:
>
> ```
> $init = function($object) use ($c) {
> $object->a = $c->get(ServiceA::class);
> $object->b = $c->get(ServiceB::class);
> }
> $service = new ReflectionLazyObjectFactory(Service::class,
> $init->bindTo(null, Service::class));
> ```
>
> If you really want to bind $this you could achieve it in a more convoluted
> way:
>
> ```
> $init = function($object) use ($c) {
> (function () use ($c) {
> $this->a = $c->get(ServiceA::class);
> $this->b = $c->get(ServiceB::class);
> })->bindTo($object)();
> }
> $service = new ReflectionLazyObjectFactory(Service::class, $init);
> ```
>
> This is inconvenient, but the need or use-case is not clear to me.
> Could you describe some use-cases where you would hand-write
> initializers like this? Do you feel that the proposal should provide
> an easier way to change $this and/or the scope?
>
> > > In practice we expect that makeInstanceLazy*() methods will not be
> > > used on fully initialized objects, and that the flag will be set most
> > > of the time, but as it is the API is safe by default.
> >
> > In the case an object does not have a destructor, it won't make a
> > difference either way, correct?
>
> Yes
>
> > >> I find it interesting that your examples list DICs as a use case for
> > >> proxies, when I would have expected that to fit ghosts better. The
> > >> common pattern, I would think, would be:
> > > The RFC didn't make it clear enough that the example was about the
> > > factory case specifically.
> >
> > Ah, got it. That makes more sense.
> >
> > Which makes me ask if the $initializer of a proxy should actually be called
&
Re: [PHP-DEV] [RFC] Lazy Objects
On Sat, Jun 15, 2024 at 7:13 PM Larry Garfield wrote:
> > In practice I expect there will be two kinds of ghost initializers:
> > - Those that just call one public method of the object, such as the
> > constructor
> > - Those that initialize everything with ReflectionProperty::setValue()
> > as in the Doctrine example in the "About Lazy-Loading strategies"
> > section
> I'm still missing an example with ::bind(). Actually, I tried to write a
> version of what I think the intent is and couldn't figure out how. :-)
>
> $init = function() use ($c) {
> $this->a = $c->get(ServiceA::class);
> $this->b = $c->get(ServiceB::class);
> }
>
> $service = new ReflectionLazyObjectFactory(Service::class, $init);
>
> // We need to bind $init to $service now, but we can't because $init is
> already registered as the initializer for $service, and binding creates a new
> closure object, not modifying the existing one. So, how does this even work?
Oh I see. Yes you will not be able to bind $this in a simple way here,
but you could bind the scope. This modified example will work:
```
$init = function($object) use ($c) {
$object->a = $c->get(ServiceA::class);
$object->b = $c->get(ServiceB::class);
}
$service = new ReflectionLazyObjectFactory(Service::class,
$init->bindTo(null, Service::class));
```
If you really want to bind $this you could achieve it in a more convoluted way:
```
$init = function($object) use ($c) {
(function () use ($c) {
$this->a = $c->get(ServiceA::class);
$this->b = $c->get(ServiceB::class);
})->bindTo($object)();
}
$service = new ReflectionLazyObjectFactory(Service::class, $init);
```
This is inconvenient, but the need or use-case is not clear to me.
Could you describe some use-cases where you would hand-write
initializers like this? Do you feel that the proposal should provide
an easier way to change $this and/or the scope?
> > In practice we expect that makeInstanceLazy*() methods will not be
> > used on fully initialized objects, and that the flag will be set most
> > of the time, but as it is the API is safe by default.
>
> In the case an object does not have a destructor, it won't make a difference
> either way, correct?
Yes
> >> I find it interesting that your examples list DICs as a use case for
> >> proxies, when I would have expected that to fit ghosts better. The common
> >> pattern, I would think, would be:
> > The RFC didn't make it clear enough that the example was about the
> > factory case specifically.
>
> Ah, got it. That makes more sense.
>
> Which makes me ask if the $initializer of a proxy should actually be called
> $factory? Since that's basically what it's doing,
Good point, $factory would be a good name for this parameter.
> and I'm unclear what it would do with the proxy object itself that's passed
> in.
Passing the factory itself as argument could be used to make decisions
based on the value of some initialized field, or on the class of the
object, or on its identity. I think Nicolas had a real use-case where
he detects clones based on the identity of the object:
```
$init = function ($object) use (&$originalObject) {
if ($object !== $originalObject) {
// we are initializing a clone
}
};
$originalObject = $reflector->newProxyInstance($init);
```
This was on ghosts, but I think it's also a valid use-case example for proxies.
> >> ReflectionLazyObjectFactory is a terrible name. Sorry, it is. :-)
> >> Especially if it's subclassing ReflectionClass. If it were its own thing,
> >> maybe, but it's still too verbose. I know you don't want to put more on
> >> the "dumping ground" fo ReflectionClass, but honestly, that feels more
> >> ergonomic to me. That way the following are all siblings:
> >>
> >> newInstance(...$args)
> >> newInstanceWithoutConstructor(...$args)
> >> newGhostInstance($init)
> >> newProxyInstance($init)
> >>
> >> That feels a lot more sensible and ergonomic to me. isInitialized(),
> >> initialized(), etc. also feel like they make more sense as methods on
> >> ReflectionObject, not as static methods on a random new class.
> >
> > Thank you for the suggestion. We will check if this fits the
> > use-cases. Moving some methods on ReflectionObject may have negative
> > performance implications as it requires creating a dedicated instance
> > for each object. Some use-cases rely on caching the reflectors for
> > performance.
> >
> > Best Regards,
> > Arnaud
>
> I'm not clear why there's a performance difference, but I haven't looked at
> the reflection implementation in, well, ever. :-)
What I meant is that creating an instance (not necessarily of
ReflectionObject, but of any class) is more expensive than just doing
nothing. The first two loops below would be fine, but the last one
would be slower. This can make an important difference in a hot path.
```
foreach ($objects as $object) {
ReflectionLazyObject::isInitialized($object);
}
$reflector = new ReflectionClass(SomeClass::class);
foreach
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Larry,
On Fri, Jun 14, 2024 at 10:18 PM Larry Garfield wrote:
> > The actual instance is allowed to escape the proxy and to create direct
> > references to itself.
>
> How? Is this a "return $this" type of situation? This could use more
> fleshing out and examples.
"return $this" will return the proxy object, but it is possible to
create references to the actual instance during initialization, either
directly in the initializer function, or in methods called by the
initializer. The "About Proxies" section discusses this a bit. I've
added an example.
> The terms "virtual" and "proxy" seem to be used interchangeably in different
> places, including in the API. Please just use one, and purge the other.
> It's confusing as is. :-) (I'd favor "proxy", as it seems more accurate to
> what is happening.)
Agreed
> Under Common Behavior, you have an example of calling the constructor
> directly, using the reflection API, but not of binding the callable, which
> the text says is also available. Please include an example of that so we can
> evaluate how clumsy (or not) it would be.
I've clarified that binding can be achieved with Closure::bind(). In
practice I expect there will be two kinds of ghost initializers:
- Those that just call one public method of the object, such as the constructor
- Those that initialize everything with ReflectionProperty::setValue()
as in the Doctrine example in the "About Lazy-Loading strategies"
section
> > After calling newLazyGhostInstance(), the behavior of the object is the
> > same as an object created by newLazyGhostInstance().
>
> I think the first is supposed be a make* call?
Thank you!
> > When making an existing object lazy, the makeInstanceLazy*() methods call
> > the destructor unless the SKIP_DESTRUCTOR flag is given.
>
> I don't quite get why this is. Admittedly destructors are rarely used, but
> why does it need to call the destructor?
The rationale is that unless specified otherwise, we must assume that
the constructor has been called on the object. Therefore we must call
the destructor before resetting the object's state entirely. See also
the Mutex example given by Tim. I've added the rationale and an
example.
In practice we expect that makeInstanceLazy*() methods will not be
used on fully initialized objects, and that the flag will be set most
of the time, but as it is the API is safe by default.
> I find it interesting that your examples list DICs as a use case for proxies,
> when I would have expected that to fit ghosts better. The common pattern, I
> would think, would be:
>
> class Service {
> public function __construct(private ServiceA $a, private ServiceB $b) {}
> }
>
> $c = some_container();
>
> $init = fn() => $this->__construct($c->get(ServiceA::class),
> $c->get(ServiceB::class));
>
> $service = new ReflectionLazyObjectFactory(Service::class, $init);
>
> (Most likely in generated code that can dynamically sort out the container
> calls to inline.)
>
> Am I missing something?
No you are right, but they must fallback to the proxy strategy when
the user provides a factory.
E.g. this will use the ghost strategy because the DIC
instantiates/initializes the service itself:
my_service:
class: MyClass
arguments: [@service_a, @service_b]
lazy: true
But this will use the proxy strategy because the DIC doesn't
instantiate/initialize the service itself:
my_service:
class: MyClass
arguments: [@service_a, @service_b]
factory: [@my_service_factory, createService]
lazy: true
The RFC didn't make it clear enough that the example was about the
factory case specifically.
> ReflectionLazyObjectFactory is a terrible name. Sorry, it is. :-)
> Especially if it's subclassing ReflectionClass. If it were its own thing,
> maybe, but it's still too verbose. I know you don't want to put more on the
> "dumping ground" fo ReflectionClass, but honestly, that feels more ergonomic
> to me. That way the following are all siblings:
>
> newInstance(...$args)
> newInstanceWithoutConstructor(...$args)
> newGhostInstance($init)
> newProxyInstance($init)
>
> That feels a lot more sensible and ergonomic to me. isInitialized(),
> initialized(), etc. also feel like they make more sense as methods on
> ReflectionObject, not as static methods on a random new class.
Thank you for the suggestion. We will check if this fits the
use-cases. Moving some methods on ReflectionObject may have negative
performance implications as it requires creating a dedicated instance
for each object. Some use-cases rely on caching the reflectors for
performance.
Best Regards,
Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Michał, Chris, On Thu, Jun 6, 2024 at 8:53 AM Michał Marcin Brzuchalski wrote: > Did you consider implementing it using some attribute? On Sun, Jun 9, 2024 at 1:24 AM Chris Riley wrote: > I'm wondering why this has been attached to the existing reflection API > instead of being a new thing in and of itself? It doesn't seem strictly > related to reflection other than currently the solutions for this rely on > reflection to work. Thank you for your feedback. Currently the lazy objects feature is designed as a low-level technical building block that libraries and frameworks can use. FFI and Fibers are examples of such features that most users do not use directly, but can benefit greatly from within libraries they use. We have considered the proposed syntaxes, such as using annotations, but these do not match the core use case of creating a lazy instance of a class without requiring cooperation from the class itself. Furthermore we believe that designing a higher-level API in the language would be considerably more difficult and could put the RFC at risk. However, it is possible to introduce a higher-level way to create lazy objects in a future RFC. Best Regards, Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Tim,
We have updated the RFC to address your feedback. Please find
additional answers below.
On Wed, Jun 5, 2024 at 8:25 PM Tim Düsterhus wrote:
> Is there any reason to call the makeLazyX() methods on an object that
> was not just freshly created with ->newInstanceWithoutConstructor()
> then?
There are not many reasons to do that. The only indented use-case that
doesn't involve an object freshly created with
->newInstanceWithoutConstructor() is to let an object manage its own
laziness by making itself lazy in its constructor:
```
class C {
public function __construct() {
new ReflectionLazyObjectFactory(C::class)->makeInstanceLazyGhost($this,
$this->init(...));
}
}
```
When drafting this API we figured that makeLazy addressed all
use-cases, and this allowed us to keep the API minimal. However,
following different feedback we have now introduced two separate
newLazyGhostInstance() and newLazyProxyInstance() methods.
> Anything I do with the object before the call to makeLazyX() will
> effectively be reverted, no?
Yes, after calling makeLazy() the object is in the same state as if it
was made lazy immediately after instantiation.
> An example showcasing the intended usage, e.g. a simplified ORM example,
> would really be helpful here.
Agreed
> > in userland by iterating on all properties via the Reflection API, and
> > using unset() in the right scope with a Closure.
> >
> > spl_object_id(), spl_object_hash(), SplObjectStorage, WeakMap,
> > WeakReference, strict equality, etc are not affected by makeLazy*().
>
> That is true for *both* makeLazyGhost(), and makeLazyProxy()?
>
> What would the following example output?
>
> $object = new MyObject();
> var_dump(spl_object_id($object));
> $r = ReflectionLazyObject::makeLazyGhost($object, function
> (MyObject $object) {
> $object2 = new MyObject();
> var_dump(spl_object_id($object2));
> return $object2;
> });
> var_dump(spl_object_id($object));
> $r->initialize();
> var_dump(spl_object_id($object));
>
> What would happen if I would expose the inner $object2 to the outer
> world by means of the super globals or by means of `use (&$out)` + `$out
> = $object2`?
We have clarified the RFC on these points
> > The intended use of makeLazyGhost() and makeLazyProxy() is to call
> > them either on an object created with
> > ReflectionClass::newInstanceWithoutConstructor(), or on $this in a
> > constructor. The latter is the reason why these APIs take an existing
> > object.
>
> Okay, that answers the question above. Technically being capable of
> calling it on an object that was not just freshly created sounds like a
> footgun, though. What is the interaction with readonly objects? My
> understanding is that it would allow an readonly object with initialized
> properties to change after-the-fact?
Good point about readonly, this is something we had overlooked. We
have updated the RFC to address this.
> >> Consider this example:
> >>
> >> class Foo {
> >> public function __destruct() { echo __METHOD__; }
> >> }
> >>
> >> class Bar {
> >> public string $s;
> >> public ?Foo $foo;
> >>
> >>public function __destruct() { echo __METHOD__; }
> >> }
> >>
> >> $bar = new Bar();
> >> $bar->foo = new Foo();
> >>
> >> ReflectionLazyObject::makeLazyProxy($bar, function (Bar $bar) {
> >> $result = new Bar();
> >> $result->foo = null;
> >> $result->s = 'init';
> >> return $result;
> >> });
> >>
> >> var_dump($bar->s);
> >>
> >> My understanding is that this will dump `string(4) "init"`. Will the
> >> destructor of Foo be called? Will the destructor of Bar be called?
> >
> > This will print:
> >
> > Foo::__destruct (during makeLazyProxy())
> > string(4) "init" (during var_dump())
> >
> > and eventually
> >
> > Bar::__destruct (when $bar is released)
>
> Okay, so only one Bar::__destruct(), despite two Bar objects being
> created. I assume it's the destructor of the second Bar, i.e. if I would
> dump `$this->foo` within the destructor, it would dump `null`?
Following your feedback we have updated the RFC to include calling
destructors in makeLazy by default. In the updated version,
Bar::__destruct is called twice: Once on the first instance during the
call to makeLazyProxy(), and another time on the second instance (the
one created in the closure) when it's released. It is not called when
the first instance is released, because it's now a proxy.
> >> Will the "revert to its pre-initialization"
> >> work properly when you have nested lazy objects? An example would
> >> probably help.
> >
> > Only the effects on the object itself are reverted. External side
> > effects are not reverted.
>
> Yes, it's obvious that external side effects are not reverted. I was
> thinking about a situation like:
>
> $a = new A();
> $b = new B();
> ReflectionLazyObject::m
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Larry,
Thank you for the feedback.
I think you got the two strategies right. However, there is a use-case
in which an object manages its own laziness by making itself lazy:
```
class C {
public function __construct() {
ReflectionLazyObject::makeLazyGhost($this, $this->init(...));
}
}
```
This one can not be addressed by a newInstance*() method since the
object to be made lazy already exists.
The makeLazyGhost() / makeLazyProxy() methods are the minimal methods
necessary to address all use-cases, but the methods you are suggesting
are a better API most of the time, so we are adding approximately this
to the proposal [1]. We are keeping them in a separate class to not
pollute ReflectionClass.
> It also suggests that perhaps the function should be using $this, not $foo,
> as it's running within the context of the object (I presume? Can it call
> private methods? I assume so since it can set private properties.)
The function is not running in the context of the object. It can only
access private members via Reflection or if the closure was bound to
the right scope by the user. This should not be an issue when the
initializer just calls a public constructor.
> In which case $object is the proxy, and gets "swapped out" for the return
> value of the $initializer on first use.
Just to be sure: $object continues to be the proxy instance after the
initializer is called, but it forwards all property accesses to the
return value of the $initializer.
[1] https://news-web.php.net/php.internals/123518
Best Regards,
Arnaud
Re: [PHP-DEV] [RFC] Lazy Objects
Hi Tim,
That's a lot of interesting feedback. I will try to answer some of
your points, and Nicolas will follow with other points.
On Tue, Jun 4, 2024 at 9:16 PM Tim Düsterhus wrote:
> - int $options = 0
>
> Not a fan of flag parameters that take a bitset, those provide for a
> terrible DX due to magic numbers. Perhaps make this a regular (named)
> parameter, or an list of enum LazyObjectOptions { case
> SkipInitOnSerialize; }?
The primary reason for choosing to represent $options as a bitset is
that it's consistent with the rest of the Reflection API (e.g.
ReflectionClass::getProperties() uses a bitset for the $filter
parameter).
I don't get your point about magic numbers since we are using
constants to abstract them.
> - skipProperty()
>
> Not a fan of the method name, because it doesn't really say what it
> does, without consulting the docs. Perhaps `skipInitializationFor()` or
> similar?
We have opted for skipInitializerForProperty()
> - setProperty()
>
> Not a fan of the method name, because it is not a direct counterpart to
> `getProperty()`. Unfortunately I don't have a better suggestion.
We have renamed setRawProperty() to setRawPropertyValue() in the RFC.
We are open to other suggestions.
We have also removed setProperty(), as we believe that there is no
use-case for it.
> - The examples should be expanded and clarified, especially the one for
> makeLazyProxy():
Agreed. We will add examples and clarify some behaviors.
> My understanding is that the $object that is passed to the first
> parameter of makeLazyProxy() is completely replaced. Is this
> understanding correct? What does that mean for spl_object_hash(),
> spl_object_id()? What does this mean for WeakMap and WeakReference? What
> does this mean for objects that are only referenced from within $object?
The object is updated in-place, and retains its identity. It is not
replaced. What makeLazyGhost() and makeLazyProxy() do is equivalent to
calling `unset()` on all properties, and setting a flag on the object
internally. Apart from setting the internal flag, this is achievable
in userland by iterating on all properties via the Reflection API, and
using unset() in the right scope with a Closure.
spl_object_id(), spl_object_hash(), SplObjectStorage, WeakMap,
WeakReference, strict equality, etc are not affected by makeLazy*().
The intended use of makeLazyGhost() and makeLazyProxy() is to call
them either on an object created with
ReflectionClass::newInstanceWithoutConstructor(), or on $this in a
constructor. The latter is the reason why these APIs take an existing
object.
The proposed patch integrates into the object handlers fallback code
path used to manage accesses to undefined properties. We implement
lazy initialization by hooking into undefined property accesses,
without impacting the fast path.
> Consider this example:
>
> class Foo {
>public function __destruct() { echo __METHOD__; }
> }
>
> class Bar {
>public string $s;
>public ?Foo $foo;
>
> public function __destruct() { echo __METHOD__; }
> }
>
> $bar = new Bar();
> $bar->foo = new Foo();
>
> ReflectionLazyObject::makeLazyProxy($bar, function (Bar $bar) {
>$result = new Bar();
>$result->foo = null;
>$result->s = 'init';
>return $result;
> });
>
> var_dump($bar->s);
>
> My understanding is that this will dump `string(4) "init"`. Will the
> destructor of Foo be called? Will the destructor of Bar be called?
This will print:
Foo::__destruct (during makeLazyProxy())
string(4) "init" (during var_dump())
and eventually
Bar::__destruct (when $bar is released)
> - What happens if I make an object lazy that already has all properties
> initialized? Will that be a noop? Will that throw? Will that create a
> lazy object that will never automatically be initialized?
All properties are unset as described earlier, and the object is
flagged as lazy. The object will automatically initialize when trying
to observe its properties.
However, making a fully initialized object lazy is not the intended use-case.
> - Cloning, unless __clone() is implemented and accesses a property.
>
> The semantics of cloning a lazy object should be explicitly spelled out
> in the RFC, ideally with an example of the various edge cases (should
> any exist).
Agreed. We are working on expanding the RFC about this
> - Before calling the initializer, properties that were not initialized
> with ReflectionLazyObject::skipProperty(),
> ReflectionLazyObject::setProperty(),
> ReflectionLazyObject::setRawProperty() are initialized to their default
> value.
>
> Should skipProperty() also skip the initialization to the default value?
> My understanding is that it allows skipping the initialization on
> access, but when initialization actually happens it should probably be
> set to a well-defined value, no?
>
> Am I also correct in my understanding that this should read "initialized
> to thei
Re: [PHP-DEV] Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM
Hi Tim, The report submitted to Apple via Feedback Assistant is not public, but here is a mirror: https://openradar.appspot.com/radar?id=5583058442911744 Best regards, Arnaud On Thu, May 23, 2024 at 8:26 PM Tim Düsterhus wrote: > > Hi > > On 5/21/24 14:39, Arnaud Le Blanc wrote: > > This is what I believe as well. FWIW I was able to reproduce the issue > > outside of PHP [2]. I've reported the bug to Apple, but I don't expect > > it to be fixed quickly, if at all. > > Is the bug report public / can you provide a link to it? > > Best regards > Tim Düsterhus
Re: [PHP-DEV] Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM
Hi Calvin, > FWIW, Cygwin and IBM i are also using wall time, because these platforms > don't do execution time. Thank you for the additional info. > IIRC, the difference between platforms with how > max_execution_time is counted doesn't seem to be documented last I checked. Currently the max_execution_time documentation has a note about that, but it only mentions Windows: > https://www.php.net/manual/en/info.configuration.php#ini.max-execution-time > On non Windows systems, the maximum execution time is not affected by system > calls, stream operations etc. Please see the set_time_limit() function for > more details. Best Regards, Arnaud
Re: [PHP-DEV] Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM
Hi Robert, > This sounds like a bug with Mac vs. a PHP issue. This is what I believe as well. FWIW I was able to reproduce the issue outside of PHP [2]. I've reported the bug to Apple, but I don't expect it to be fixed quickly, if at all. > That being said, the > biggest issue between changing these clocks is that ITIMER_PROF is > (practically, but not explicitly) monotonic. ITIMER_REAL can go > backwards (NTP clock adjustments) which might have interesting > side-effects if the clock is adjusted. Good point. Do you have references about ITIMER_REAL using a non-monotonic clock, besides the lack of specification regarding the clock? Based on experiments and the code [1], I think that ITIMER_REAL uses a monotonic clock on MacOS. It's not ideal to rely on that, and we should use a better specified timer mechanism if we eventually switch to wall-clock time on all platforms, but it seems reasonable as a workaround for the ITIMER_PROF bug on MacOS/Apple Silicon. > The problem might actually be using ITIMER_PROF, which "Measures CPU > time used by the process, including both user space and kernel space" > and usage of sockets/threads might give an "accelerated" value while > maybe ITIMER_VIRTUAL is the one we should be using since it "Measures > CPU time used by the process (user space)" which won't count kernel > timings. Unfortunately ITIMER_VIRTUAL is not really useful as a max_execution_time implementation as it will basically never fire in a syscall-heavy workload. E.g. after replacing ITIMER_PROF by ITIMER_VIRTUAL in [2], the program runs for well over the specified time before receiving a signal, despite consuming a considerable amount of resources. [1] https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/kern_time.c#L432 [2] https://gist.github.com/arnaud-lb/012195a2fe4d3a2c1bff530a73ae6b11 Best Regards, Arnaud
[PHP-DEV] Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM
Hi internals, There is an issue with max_execution_time on MacOS, probably only MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1]. max_execution_time is implemented with setitimer(ITIMER_PROF) on this platform, and the SIGPROF signal is delivered too early for some reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel Kress opened a PR [3]. Are there objections against switching to ITIMER_REAL on MacOS (Apple Silicon only) in all currently supported PHP versions? (next 8.2.x, 8.3.x, 8.x) Apart from fixing the issue, this would introduce the following minor breaking changes: - max_execution_time on MacOS would be based on wall-clock time rather than CPU time, so the time spent in I/O, sleep(), and syscalls in general would count towards the max_execution_time - The SIGALRM signal would be used instead of the SIGPROF signal (using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts with profilers). As noted by Dmitry, it also conflicts with sleep() on some platforms, however this should be safe on MacOS. Currently max_execution_time is based on wall-clock time on ZTS and Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time can also be used when building with --enable-zend-max-execution-timers. Máté proposed to add a wall-clock based timeout in the past [2] but the discussion has stalled. Any thoughts about eventually switching other platforms to wall-clock timeouts in the next 8.x ? TL;DR: - Any objection about using wall-clock max_execution_time and SIGALRM on MacOS Apple Silicon in all supported versions? - Thoughts about using wall-clock timeouts on all platforms in the next 8.x ? [1] https://github.com/php/php-src/issues/12814 [2] https://github.com/php/php-src/pull/6504 [3] https://github.com/php/php-src/pull/13567 Best Regards, Arnaud
Re: [PHP-DEV] Requiring GPG Commit Signing
On Tue, Apr 2, 2024 at 4:16 PM Derick Rethans wrote: > What do y'all think about requiring GPG signed commits for the php-src > repository? > +1
Re: [PHP-DEV] XSLTProcessor recursion limit
Hi Niels, On Sat, Mar 16, 2024 at 5:49 PM Niels Dossche wrote: > Hi internals > > Based on https://bugs.php.net/bug.php?id=71571 I've opened a PR to > implement two new properties for XSLTProcessor: maxTemplateDepth and > maxTemplateVars. The reasoning behind this is that large templates with > lots of recursion and/or variables can bump against the default recursion > limit set by the libxslt library. > > PR link: https://github.com/php/php-src/pull/13731 > > I used an adapted version of > https://github.com/nikic/popular-package-analysis to download every > package that satisfies the search term "xsl". Then I checked if there are > any classes that extend XSLTProcessor and have the maxTemplateDepth or > maxTemplateVars field. > None do, and as such no package in packagist will observe a BC break > because of this PR. > > One sad detail however, is that you can set the recursion limit so high > that you can exhaust the stack space, which will crash the process with a > segfault. In fact, you can hit this already right now without the PR, using > XSL. I.e. you can create a very deep VM re-entry that won't cause the stack > limit protection to kick in, and then start a recursive XSL template > processing that does not hit XSL's limit, but exhausts the remaining stack > space. Note that as soon as you do callbacks however, the stack limit > protection _will_ kick in. > I tried to check if it's possible to prevent this, but the stack limit > check would need to happen inside the libxslt library, so I don't think > it's possible. > > Let me know if there are any complaints about this. If no complaints, I'll > merge this in 2 weeks or so. > > Kind regards > Niels > For PCRE there is a pcre.recursion_limit setting [1] with the same issue. This is fine because that's a convenience feature (not a security one) and it is useful by catching most cases of uncontrolled recursion in practice. The default value is not changed by this PR (so there is no risk of BC break by lowering the limit), and appears to be low enough to prevent stack exhaustions on a default 2MiB stack on Linux. +1 for this [1] https://www.php.net/manual/en/pcre.configuration.php#ini.pcre.recursion-limit Kind regards, Arnaud
Re: [PHP-DEV] New gc_status() fields, and WeakMap cycle collection
I agree that we may not consider this a BC break, but I'm asking in case people rely on the current behavior or if the change is more controversial than I thought. To be fair they work as specified in the RFC, and are actually weak as they do not explicitly retain their keys, but they need explicit support from the GC to be able to collect some types of cycles. WeakMap implementations based on WeakReferences tend to have this limitation. For instance, the Java documentation for WeakHashMap has this warning: "The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded". On Fri, Jul 7, 2023 at 4:03 PM Larry Garfield wrote: > > On Fri, Jul 7, 2023, at 1:09 PM, Arnaud Le Blanc wrote: > > Hi everyone, > > > > I'm considering merging the following two PRs: > > > > https://github.com/php/php-src/pull/11523: Exposes the time spent in > > the cycle collector via new gc_status() fields. > > https://github.com/php/php-src/pull/10932: Allows the GC to collect > > certain types of cycles involving WeakMaps. > > > > The second one introduces a minor BC break. > > > > If there are no objections, I plan to merge these PRs prior to the > > feature freeze. > > > > Best regards, > > Arnaud > > For the second, doesn't that translate to "WeakMaps are currently not > actually Weak, and thus don't work at all?" I don't quite know how that > would be a BC break to fix... > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] New gc_status() fields, and WeakMap cycle collection
Hi everyone, I'm considering merging the following two PRs: https://github.com/php/php-src/pull/11523: Exposes the time spent in the cycle collector via new gc_status() fields. https://github.com/php/php-src/pull/10932: Allows the GC to collect certain types of cycles involving WeakMaps. The second one introduces a minor BC break. If there are no objections, I plan to merge these PRs prior to the feature freeze. Best regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Preventing stack overflows
On 07/10/2022 16:14, Yasuo Ohgaki wrote:
The root cause that users cannot understand what happened is this:
$ php -n -r 'set_error_handler(function ($severity,$message, $filename,
$lineno) { throw new ErrorException($message, 0, $severity, $filename,
$lineno); }); function f() { f(); } f();'
Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to
allocate 262144 bytes) in Command line code on line 1
When a fatal error happens, PHP does not allow a stack dump. Very old
PHP allowed users to catch E_ERROR by user error handler, but it is
disabled to prevent users from shooting their own foot.
I suppose allowing users to catch "only one" fatal error would solve
many issues as well as infinite recursion.
Fatal errors definitely need improvements.
Stack overflows are even worth, as the process is terminated by the system:
$ php test.php
Segmentation fault
With the proposed change, an exception is throw before it happens:
$ php test.php
Fatal error: Uncaught Error: Maximum call stack size of 28672 bytes
reached. Infinite recursion? in test.php:8
Stack trace:
#0 test.php(8): C->__destruct()
#1 test.php(8): C->__destruct()
#2 test.php(8): C->__destruct()
...
#7 {main}
thrown in test.php on line 8
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Preventing stack overflows
Hi internals, I would like to propose a way to detect stack overflows before they happen, with the goal of improving debugability. Stack overflows are caused by excessive stack usage, typically due to deep recursions. Recursion in PHP doesn't use recursion internally, but some constructs still do, and it is not always possible to avoid without dramatically increasing complexity or degrading performances. Some examples of these constructs can be found at [1][2][3]. Programs that overflow the stack are terminated with a SIGSEGV, and the user is left with no hint as to which code is causing the issue. This makes debugging difficult. Xdebug makes debugging easier by throwing an exception when the function call nesting level exceeds a certain limit [4], which is very useful. However, this can be overly limiting because this does not discriminate between the calls that actually use the stack and those that do not. Nikita proposed an other solution a while ago [5] that limits in terms of VM reentrances, so that only the constructs that actually cause internal recursion count towards the limit. One issue is that not all VM frames will consume the same amount of stack, and the maximum stack size depends on the system, so it's hard to determine a good default value for the limit that is not overly limiting. I would like to propose an alternative [6] that limits in terms of stack size. One issue is that it increases the internals complexity a bit, but this would make debugging easier without limiting recursion too much compared to what the system would allow. Any thoughts? [1] https://bugs.php.net/bug.php?id=64196 [2] https://bugs.php.net/bug.php?id=75509 [3] https://github.com/php/php-src/issues/8315 [4] https://xdebug.org/docs/develop#max_nesting_level [5] https://externals.io/message/108348 [6] https://github.com/php/php-src/pull/9104 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Vote] Auto-capture closures
On vendredi 1 juillet 2022 16:04:30 CEST Larry Garfield wrote: > Greetings, Internalians. > > The vote for auto-capture closures is now open. It will run until 15 July. > > https://wiki.php.net/rfc/auto-capture-closure Hi Internals, The RFC has been declined with a vote of 27 in favor and 16 against. Thank you for the votes and the feedbacks. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Vote] Auto-capture closures
On mardi 5 juillet 2022 09:27:34 CEST Marco Pivetta wrote: > This is fine, but it creates a bidirectional dependency between components > where the dependency graph was previously (in theory) acyclic. > > This will become "someone else's problem" 😬 I don't think that the Optimizer conceptually depends on the Compiler currently. It does include `Zend/zend_compile.h`, but that's only for the types defined in this header. It needs these types because its responsibility is to transform data structures produced by the compiler (op_arrays), or in this case, to produce meta data about these structures. These types can be trivially moved to an other header. It doesn't call any compiler code. Also, this doesn't introduces circular includes involving Zend/zend_compile.h. Does it seems ok to you ? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Vote] Auto-capture closures
Hi, On lundi 4 juillet 2022 11:47:03 CEST Nicolas Grekas wrote: > I'm just wondering if there could be a way to enable the auto-capture > optimization for arrow functions. Having three kinds of rules for capturing > feels too much, especially when there are that subtles for short closures. > Maybe a deprecation could be possible for affected arrow functions? Yes. I will propose this after this RFC. This is not included in the RFC because the optimization rarely have an impact on Arrow Functions (they rarely bind variables). -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Vote] Auto-capture closures
Hi, On lundi 4 juillet 2022 12:04:59 CEST Marco Pivetta wrote: > I ended up voting NO due to the fact that there is no mention of `$this` > behavior changes ( https://externals.io/message/117888#117889 ) : I also > disagree with NikiC's stance on this being the same construct as before ( > https://externals.io/message/117888#117897 ), so I really wanted to get rid > of this constant memleak risk that we have in closures. I should have made > my stance stronger, instead of letting the discussion thread die, but time > is what it is. Forgot to add this in Future Scope, sorry. The RFC does NOT change the behavior of `$this`, so as to keep it small. This is something I personally want, but it can be done after this RFC, and the change could be applied to all function kinds. > Unclear (to me) after looking at > https://gist.github.com/arnaud-lb/d9adfbf786ce9e37c7157b0f9c8d8e13, is > whether the optimizations are applied at compile time. It looks like the > changes are inside the `Zend/Optimizer/`, and `zend_compile.c`, so perhaps > the benchmarks are probably just worded in a confusing way. Sorry for the confusion. I confirm that the optimizations are applied at compile time. The benchmarks measure the runtime effect of these optimizations. > Important to note how `Zend/zend_compile.c` now depends on `Optimizer/`, > which is a potential design issue. The Optimizer was moved to `Zend/Optimizer` earlier so that in the long term it could be used by the compiler pipeline directly (rather than as part of caching). There are clear benefits in reusing the Optimizer here. Duplicating or reimplementing live variable analysis would increase maintenance cost, whereas by reusing the Optimizer's implementation we take profit of a fast and well tested implementation. Greets, -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi Björn, On Wed, Jun 29, 2022 at 8:09 PM Björn Larsson via internals < internals@lists.php.net> wrote: > Would it be an option to include a "Future scope" with the features: > - Explicit capture that list only the variables to be captured by value > or reference, nothing else. > - Extending the traditional anonymous function with use(*) for capturing > everything. > > Anyway, hope this passes for PHP 8.2! > Thank you for the suggestion. The RFC now includes a Future Scope section. The extension of traditional anonymous functions is discussed separately in the "Alternative implementations" section. Regards,
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi Rowan, Since this still has a small chance of breaking existing code, we preferred to exclude arrow functions from this change, for now. We have added this in Future Scope. This is something we could do in a follow-up RFC. Regards,
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi Guilliam, Thank you for noticing. The PR is now fully in sync with the RFC (no arrow function changes, no explicit use support). The RFC also now clarifies that arrow functions are not changed by the RFC.
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On jeudi 30 juin 2022 18:29:44 CEST Guilliam Xavier wrote: > Ah? Sorry, I had interpreted > https://github.com/php/php-src/pull/8330/files#diff-85701127596aca0e597bd796 > 1b5d59cdde4f6bb3e2a109a22be859ab7568b4d2R7318-R7320 as "capture the > *minimal* set of variables for *both* arrow functions and short closures", > but I was wrong? No, you are right, the PR changes arrow functions too. But in the RFC we decided to not touch the arrow functions for now. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi, On jeudi 30 juin 2022 16:18:44 CEST Robert Landers wrote: > Are > optimizations going to be applied to single-line arrow functions (I > didn't see that in the RFC, but I admittedly didn't look that hard and > I vaguely remember reading something about it in one of these > threads)? If so, it will probably change some behaviors in existing > applications if they were relying on it. Perhaps static analysis tools > can detect this and inform the developer. It is not planned to change the behavior of arrow functions in this RFC. This optimization is less important for arrow functions because they don't usually assign variables. This could be a follow up RFC though. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi,
On jeudi 30 juin 2022 00:31:44 CEST Dan Ackroyd wrote:
> On Wed, 29 Jun 2022 at 18:30, Larry Garfield wrote:
> > The conversation has died down, so we'll be opening the vote for this
> > tomorrow.
> I think I've just thought of a problem with the optimization bit of
> 'not capturing variables if they are written to before being used
> inside the closure'.
>
> Imagine some code that looks like this:
>
> // Acquire some resource e.g. an exclusive lock.
> $some_resource = acquire_some_resource();
>
> $fn = fn () {
> // Free that resource
> $some_resource = null;
> }
>
> // do some stuff that assumes the exclusive
> // lock is still active.
>
> // call the callback that we 'know' frees the resource
> $fn();
>
> That's a not unreasonable piece of code to write even if it's of a
> style many people avoid. I believe in C++ it's called "Resource
> acquisition is initialization", though they're trying to change the
> name to "Scope-Bound Resource Management" as that is a better
> description of what it is.
I feel that the RAII pattern aka SBRM / Scope-Bound Resource Management is not
relevant in PHP context, and I don't believe that it's commonly used in PHP or
in garbage collected language.
Also, in this particular code example, using an explicit fclose() would be
better in every way, including legibility and reliability, so this doesn't
appear to be realistic code.
Because of this, I don't think that we should be taking decisions on this
feature based on this use case.
I've used the RAII pattern in PHP to manage temporary files, as a best-effort
way to remove them (in a destructor) when they are not used anymore. However I
would not rely on this for anything more critical or anything that requires
predictability in resource release timing.
RAII is useful in C++ because memory is managed manually. This is not the case
in PHP.
It's also useful in C++ to manage other kinds of resources such as file
pointers or locks. In PHP it would be dangerous because you don't
realistically control the lifetime of values, so you also don't control the
timing at which the resources are closed. It's too easy to extend the lifetime
of a value accidentally.
One way the lifetime of a value could be extended is via a reference cycle.
These are easy to introduce and difficult to prevent or observe (e.g. in a
test or in an assertion). An other way would be by referencing the value
somewhere else. You can not guarantee that the lifetime of a value is
unaffected after passing it to a function.
In C++ it's different because no code would implicitly keep a reference to a
variable passed to it unless it was part of that code's contract, or unless
the variable was refcounted.
Another factor that makes RAII un-viable in PHP is that the order of the
destructor calls is unspecified. Currently, if multiple objects go out of
scope at the same time, they happen to be called in a FIFO order, which is not
what is needed when using the RAII pattern [0][1].
I think that RAII can only realistically be used in a non-managed, non-
refcounted, non-GC language. GC or reference counting should not be used to
manage anything else than memory allocation.
Other languages typically have other ways to explicitly manage the lifetime of
resources. Go has `defer()` [2]. Python has context managers / `with` [3], C#
has `using` [4]. `with` and `using` can be implemented in userland in PHP.
Because of all these reasons, I don't think that RAII in PHP is practical or
actually used. So I don't think that we should be taking decisions on Short
Closures based on this use case.
> With the optimization in place, that code would not behave
> consistently with how the rest of PHP works
There exist no circumstance in PHP in which the existence of the statement
`$a = null` would extend the lifetime of the value bound to `$a`.
[0] Destructor order PHP: https://3v4l.org/iGAPj
[1] Destructor order C++: https://godbolt.org/z/f78Pa9j69
[2] https://go.dev/doc/effective_go#defer
[3] https://docs.python.org/3/reference/compound_stmts.html#with
[4] https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/
keywords/using-statement
Cheers,
--
Arnaud Le Blanc
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On lundi 13 juin 2022 15:36:26 CEST Rowan Tommins wrote: > > Auto-capture in PHP is by-value. This makes this impossible. It also makes > > explicit declarations non-necessary and much less useful. > > > > Live-variable analysis is mentioned in as part of implementation details. > > It should not be necessary to understand these details to understand the > > behavior of auto-capture. > > As noted in my other e-mail, by-value capture can still have side > effects, so users may still want to ensure that their code is free of > such side effects. My choice of words in this reply was inaccurate when I said "In these languages it is easy to accidentally override/bind a variable from the parent scope by forgetting a variable declaration.", since "override" can be interpreted in different ways. What I meant here is that it is not possible to accidentally bind a variable on the parent scope. This is actually impossible unless you explicitly capture a variable by-reference. Do you agree with this ? Possible side-effects via object mutations are documented in the "No unintended side-effects" section of the RFC. This assumes that property assignments or method calls to captured objects would be intended, since these assignments/calls would result in an error if the variable was not defined and not captured. Do you have examples where assignments/calls would non- intendedly cause a side effect, with code you would actually write ? > As noted in my other e-mail, by-value capture can still have side > effects, so users may still want to ensure that their code is free of > such side effects. There are two ways for a closure to have a side-effect (already documented in the RFC) : - The closure explicit captures a variable by reference, and bind it - The closure mutates a value accessed through a captured variable. Mutable values include objects and resources, but NOT scalars or arrays (since they are copy-on-write). In the first case, this is entirely explicit. In the second case, the only thing you need do understand is that if you access a variable you did not define, the variable is either undefined or comes from the declaring scope. Accessing undefined variables is an error, so it must come from the declaring scope. Your example uses isset(), which is valid code in most circumstances, but as you said it's not particularly good code. Do you have other examples that come to mind ? > Currently, the only way to do so is to understand the "implementation > details" I'm willing to make changes if that's true, because I definitely don't want this to be the case. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi Dan,
On lundi 13 juin 2022 19:49:10 CEST Dan Ackroyd wrote:
> > Auto-capture in PHP is by-value. This makes this impossible. It also makes
> > explicit declarations non-necessary and much less useful.
>
> Separating off some pedantism from the hopefully constructive comment,
>
> I think some of the words in the RFC are a bit inaccurate:
> > A by-value capture means that it is not possible to modify any variables
> > from the outer scope:
> >
> > Because variables are bound by-value, the confusing behaviors often
> > associated with closures do not exist.
> >
> > Because variables are captured by-value, Short Closures can not have
> > unintended side effects.
> Those statements are true for scalar values. They are not true for objects:
This is shown in the "No unintended side-effects" section of the RFC.
I agree that the choice of words is inaccurate, as "modify any variable" could
be interpreted not only as "bind a variable", but also as "mutate a value".
The section you have quoted is meant to show how by-value capture, which is
the default capture mode in all PHP closures, is less error prone than by-
variable/by-reference capture, by a huge margin. Especially since variable
bindings do not have side-effects unless a variable was explicitly captured
by-reference. Do you agree with this ?
The "No unintended side-effects" section assumes that property assignments to
captured variables are intended side-effects. In your example, the programmer
intended to have a side effect because `$a` can only come from the declaring
scope (the code would result in an error otherwise) :
> $a = new Foo('bar');
> $f = fn() {
> $a->value = 'explicit scope is nice';
> };
Do you have an example where the intent would be less obvious ? With code you
would actually write ?
Cheers,
--
Arnaud Le Blanc
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On samedi 11 juin 2022 23:14:28 CEST Rowan Tommins wrote:
> My main concern is summed up accidentally by your choice of subject line
> for this thread: is the proposal to add *short closure syntax* or is it
> to add *auto-capturing closures*?
The proposal is to extend the Arrow Functions syntax so that it allows
multiple statements. I wanted to give a name to the RFC, so that we could
refer to the feature by that name instead of the longer "auto-capture multi-
statement closures". But the auto-capture behavior is an important aspect we
want to inherit from Arrow Functions.
> As such, I think we need additional features to opt
> back out of capturing, and explicitly mark function- or block-scoped
> variables.
Currently the `use()` syntax co-exists with auto-capture, but we could change
it so that an explicit `use()` list disables auto-capture instead:
```php
fn () use ($a) { } // Would capture $a and disable auto-capture
fn () use () { } // Would capture nothing and disable auto-capture
```
> On the other hand, "auto-capturing" could be seen as a feature in its
> own right; something that users will opt into when it makes sense, while
> continuing to use explicit capture in others. If that is the aim, the
> proposed syntax is decidedly sub-optimal: to a new user, there is no
> obvious reason why "fn" and "function" should imply different semantics,
> or which one is which. A dedicated syntax such as use(*) or use(...)
> would be much clearer. We could even separately propose that "fn" and
> "function" be interchangeable everywhere, allowing combinations such as
> "fn() use(...) { return $x; }" and "function() => $x;"
Unfortunately, Arrow Functions already auto-capture today, so requiring a
`use(*)` to enable auto-capture would be a breaking change.
> I don't find the comparison to a foreach loop very convincing. Loops are
> still only accessing variables while the function is running, not saving
> them to be used at some indeterminate later time.
Do you have an example where this would be a problem?
> This is also where comparison to other languages falls down: most
> languages which capture implicitly for closures also merge scopes
> implicitly at other times - e.g. global variables in functions; instance
> properties in methods; or nested block scopes. Generally they also have
> a way to opt out of those, and mark a variable as local to a function or
> block; PHP does not, because it has always required an opt *in*.
These languages capture/inherit in a read-write fashion. Being able to scope a
variable (opt out of capture) is absolutely necessary otherwise there is only
one scope.
In these languages it is easy to accidentally override/bind a variable from
the parent scope by forgetting a variable declaration.
Auto-capture in PHP is by-value. This makes this impossible. It also makes
explicit declarations non-necessary and much less useful.
> Which leads me back to my constructive suggestion: let's introduce a
> block scoping syntax (e.g. "let $foo;") as a useful feature in its own
> right, before we introduce short closures.
I like this, especially if it also allows to specify a type. However, I don't
think it's needed before this RFC.
> As proposed, users will need to have some idea of what "live variable
> analysis" means, or add dummy assignments, if they want to be sure a
> variable is actually local. With a block scoping keyword, they can mark
> local variables explicitly, as they would in other languages.
Live-variable analysis is mentioned in as part of implementation details. It
should not be necessary to understand these details to understand the behavior
of auto-capture.
I've updated the "Auto-capture semantics" section of the RFC.
Regards,
--
Arnaud Le Blanc
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On dimanche 12 juin 2022 20:05:02 CEST Dan Ackroyd wrote: > On Thu, 9 Jun 2022 at 17:34, Larry Garfield wrote: > > That RFC didn't fully go to completion due to concerns over the > > performance impact > I don't believe that is an accurate summary. There were subtle issues > in the previous RFC that should have been addressed. Nikita Popov > wrote in https://news-web.php.net/php.internals/114239 > It would produce a better discussion if the RFC document either said > how those issues are resolved, or detail how they are still > limitations on the implementation. > To be clear, I don't fully understand all those issues myself (and I > have just enough knowledge to know to be scared to look at that part > of the engine) but my understanding is that the concerns are not about > just performance, they are deep concerns about the behaviour. Thank you for pointing this out. Nikita was referring to side-effects of capturing too much variables, and suggested to make the capture analysis behavior explicitly unspecified in the RFC so that it could be changed (optimized) later. The new version of the RFC does the optimization. Following your comment, I have clarified a few things in the "Auto-capture semantics" section. This includes a list of way in which these effects can be observed. These are really marginal cases that are not relevant for most programs. Cheers -- Arnaud Le Blanc -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On lundi 13 juin 2022 12:28:17 CEST Robert Landers wrote:
> From a maintainer and code review aspect, I prefer the longer syntax
> because it is 100% clear on which variables are being closed over and
> utilized in the anonymous function. fn($x) => $x + $y is pretty clear
> that $y is being pulled in from an outer scope but if you start
> getting into longer ones, it can get non-obvious pretty quickly...
>
> $func = fn($x) {
> $y[] = $x;
> // do some stuff
> return $y;
> }
>
> If $y is pulled from the outside scope, it may or may not be
> intentional but hopefully, it is an array. If anyone uses the name $y
> outside the lambda, this code may subtly break.
This is true for any function that uses the array-append operator on an
undefined variable.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
On dimanche 12 juin 2022 19:54:06 CEST Mark Baker wrote: > Did many of those closures use "pass by reference" in the use clause, > because that's one real differentiator between traditional closures and > short lambdas. There's also the fact that use values are bound at the > point where the closure is defined, not where it's called (if they even > exist at all at that point), although that's probably more difficult to > determine. Please note that auto-capture binds variables at function declaration. This is the case in Arrow Functions, and is inherited by this RFC. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3
Hi,
On jeudi 9 juin 2022 18:46:53 CEST Marco Pivetta wrote:
> ## nesting these functions within each other
>
> What happens when/if we nest these functions? Take this minimal example:
>
> ```php
> $a = 'hello world';
>
> (fn () {
> (fn () {
> echo $a;
> })();
> })();
> ```
Capture bubbles up. When an inner function uses a variable, the outer function
in fact uses it too, so it's captured by both functions, by-value.
This example prints "hello world": The inner function captures $a from the
outer function, which captures $a from its declaring scope.
This is equivalent to
```php
(function () use ($a) {
(function () use ($a) {
echo $a;
})();
})();
```
> ## capturing `$this`
>
> In the past (also present), I had to type `static fn () => ...` or `static
> function () { ...` all over the place, to avoid implicitly binding `$this`
> to a closure, causing hidden memory leaks.
>
> Assuming following:
>
> * these new closures could capture `$this` automatically, once detected
> * these new closures can optimize away unnecessary variables that aren't
> captured
>
> Would that allow us to get rid of `static fn () {` declarations, when
> creating one of these closures in an instance method context?
It would be great to get rid of this, but ideally this would apply to Arrow
Functions and Anonymous Functions as well. This could be a separate RFC.
--
Arnaud Le Blanc
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] instance version of match ?
On mardi 29 mars 2022 15:33:41 CEST Thomas Nunninger wrote:
> Hi,
>
> Am 29.03.22 um 14:34 schrieb Rowan Tommins:
> > On 29/03/2022 11:59, Robert Landers wrote:
> >> $object instanceof AnotherInterface => 'bar',
> >>
> >> We can see that `SomeInterface` will resolve the interface and not the
> >> constant.
> >
> > Yeah, the instanceof operator is magic in that regard - it has a special
> > parsing rule to consume the next token and avoid it being evaluated as a
> > constant.
> >
> >> I think what they are proposing is that when the match is an object,
> >> and the branches are class/interface/etc names, it should just do an
> >> `instanceof` operation instead of a value-equals operation.
> >
> > That wouldn't work, because the type of value passed to match() can vary
> > at run-time, but you'd need to compile the expression one way or the
> > other.
> >
> > If it did work, it would be extremely confusing:
> >
> > function example(string|object $input) {
> >
> > return match($input) {
> >SomeClass => 'found class',
> > SOME_CONSTANT => 'found constant',
> > };
> >
> > }
> > var_dump( example(new SomeClass) );
> > var_dump( example(SOME_CONSTANT) );
> >
> > Do both of those matches succeed? What if I set `const SomeClass =
> > 'hello';`?
> >
> >
> > So unfortunately we need some extra syntax to say that something should
> > be an instanceof check, and therefore a class name.
>
> While I liked the intention of Karoly, I did not like the proposed magic.
>
> Would it be an idea (and possible) to extend the syntax somehow like:
>
> $result = match ($value) {
> instanceof MyObject => ...,
>
> >= 42 => ...,
>
> !== 5 => ...
> };
>
> to be equivalent to:
>
> $result = match (true) {
> $value instanceof MyObject => ...,
> $value >= 42 => ...,
> $value !== 5 => ...
> };
>
>
> Regards
> Thomas
I like this.
The pattern matching RFC may also cover this:
https://wiki.php.net/rfc/pattern-matching
-- Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] Arbitrary string interpolation
Hi Ilija
I find that sprintf() is easier to read in most cases. One reason for this is
that the text is separated from the code. It's also easier to review for
humans and linters.
The strtoupper example would look like this with sprintf:
$world = 'world';
echo sprintf('Hello %s!', strtoupper($world));
Longer examples can be nicely split in multiple lines:
echo sprintf(
'Received HTTP status code %d (reason phrase: %s)',
$response->getStatusCode(),
$response->getReasonPhrase(),
);
And this also works with heredoc:
echo sprintf(
<<<'HTML'
%s
HTML,
htmlspecialchars($title),
);
-- Arnaud
On jeudi 17 mars 2022 23:27:30 CET Ilija Tovilo wrote:
> Hi everyone
>
> I'd like to start discussion on a new RFC for arbitrary string
> interpolation. https://wiki.php.net/rfc/arbitrary_string_interpolation
>
> Let me know what you think.
>
> Ilija
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Disabling the GC during shutdown
Hi, This bug may be related (and has a reproducing script) : https://bugs.php.net/bug.php?id=63734 On Sat, Jun 22, 2013 at 4:41 AM, Andi Gutmans wrote: > >-Original Message- > >From: Stas Malyshev [mailto:smalys...@sugarcrm.com] > >Sent: Thursday, June 20, 2013 8:20 PM > >To: Anthony Ferrara > >Cc: Laruence; internals@lists.php.net > >Subject: Re: [PHP-DEV] Disabling the GC during shutdown > > > >> However, that's not really fixing the situation either, as the zval is > >> still getting nuked (but only partially). > > > >If there's a memory overwrite or use-after-free is going on, this patch > is not a > >complete solution - it relies on the fact that "bad" data will be always > out of > >range of "good" data. I see no way to ensure that - so if there's an > overwrite > >that writes garbage inside the object there will be situations where the > >garbage looks exactly like a valid object ID and it will still crash, but > it would > >be significantly harder to reproduce. > >So I think before patching it we need to get to the root cause and figure > out > >why it happens and what causes it, instead of partially fixing the > symptom > > I agree with that. I think it'd be a mistake to submit any patch without > us understanding root cause. > We may cover up a bug which will resurface elsewhere... > Hopefully you can find a way to pin it down. > > Thanks for putting this much effort into it! > Andi > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > >
[PHP-DEV] [PATCH] Bug #60139 closure memory leaks
Hi, Closures can hold references to variables, but don't expose them to the garbage collector. The GC is unable to find cycles in which a closure is involved, and this can create memory leaks. I've reported this problem in https://bugs.php.net/bug.php?id=60139 and posted a patch. Can someone please review it ? This fixes the problem by returning the closure's closed variables and this_ptr from get_properties (most of this has been copied from SplObjectStorage's get_properties). I would like to commit in 5.3 as well. Best Regards, -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Ternary operator performance improvements
Hi, Le Monday 17 October 2011 15:07:30, Alain Williams a écrit : > On Fri, Oct 14, 2011 at 08:08:56PM +0200, Arnaud Le Blanc wrote: > > Hi, > > > > I've already posted this patch and it has since been reviewed and > > improved. I'm re-posting it for discussion before eventually commiting > > it. > > > > The ternary operator always copies its second or third operand, which is > > very > > > slow compared to an if/else when the operand is an array for example: > Is that why the following does not work as I expected: > > $dbh = $how == 'r' ? (&$dbh_r) : (&$dbh_w); > > $dbh is NOT a reference to $dbh_r or $dbh_w. This is expected; http://docs.php.net/manual/en/language.operators.comparison.php explains it: > Please note that the ternary operator is a statement, and that it doesn't evaluate to a variable, but to the result of a statement. This is why you can't assign the result of the ternary operator by reference. The patch doesn't change this. Best regards, -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Ternary operator performance improvements
Hi,
I've already posted this patch and it has since been reviewed and improved.
I'm re-posting it for discussion before eventually commiting it.
The ternary operator always copies its second or third operand, which is very
slow compared to an if/else when the operand is an array for example:
$a = range(0,9);
// this takes 0.3 seconds here:
for ($i = 0; $i < 500; ++$i) {
if (true) {
$b = $a;
} else {
$b = $a;
}
}
// this takes 3.8 seconds:
for ($i = 0; $i < 500; ++$i) {
$b = true ? $a : $a;
}
I've tried to reduce the performance hit by avoiding the copy when possible
(patch attached).
Benchmark:
Without patch: (the numbers are the time taken to run the code a certain
amount of times)
$int = 0;
$ary = array(1,2,3,4,5,6,7,8,9);
true ? 1 : 00.124
true ? 1+0 : 0 0.109
true ? $ary : 0 2.020 !
true ? $int : 0 0.103
true ? ${'ary'} : 0 2.290 !
true ?: 0 0.091
1+0 ?: 00.086
$ary ?: 0 2.151 !
${'var'} ?: 0 2.317 !
With patch:
true ? 1 : 00.124
true ? 1+0 : 0 0.195
true ? $ary : 0 0.103
true ? $int : 0 0.089
true ? ${'ary'} : 0 0.103
true ?: 0 0.086
1+0 ?: 00.159
$cv ?: 00.090
${'var'} ?: 0 0.089
The array copying overhead is eliminated. There is however a slowdown in some
of the cases, but overall there is no completely unexpected performance hit as
it is the case currently.
What do you think ? Is there any objection ?
Best regards,
diff --git a/Zend/micro_bench.php b/Zend/micro_bench.php
index 87a1b15..7052588 100644
--- a/Zend/micro_bench.php
+++ b/Zend/micro_bench.php
@@ -202,6 +202,35 @@ function read_str_offset($n) {
}
}
+function issetor($n) {
+ $val = array(0,1,2,3,4,5,6,7,8,9);
+ for ($i = 0; $i < $n; ++$i) {
+ $x = $val ?: null;
+ }
+}
+
+function issetor2($n) {
+ $f = false; $j = 0;
+ for ($i = 0; $i < $n; ++$i) {
+ $x = $f ?: $j + 1;
+ }
+}
+
+function ternary($n) {
+ $val = array(0,1,2,3,4,5,6,7,8,9);
+ $f = false;
+ for ($i = 0; $i < $n; ++$i) {
+ $x = $f ? null : $val;
+ }
+}
+
+function ternary2($n) {
+ $f = false; $j = 0;
+ for ($i = 0; $i < $n; ++$i) {
+ $x = $f ? $f : $j + 1;
+ }
+}
+
/*/
function empty_loop($n) {
@@ -318,4 +347,12 @@ read_hash(N);
$t = end_test($t, '$x = $hash[\'v\']', $overhead);
read_str_offset(N);
$t = end_test($t, '$x = $str[0]', $overhead);
+issetor(N);
+$t = end_test($t, '$x = $a ?: null', $overhead);
+issetor2(N);
+$t = end_test($t, '$x = $f ?: tmp', $overhead);
+ternary(N);
+$t = end_test($t, '$x = $f ? $f : $a', $overhead);
+ternary2(N);
+$t = end_test($t, '$x = $f ? $f : tmp', $overhead);
total($t0, "Total");
diff --git a/Zend/tests/ternary.phpt b/Zend/tests/ternary.phpt
new file mode 100644
index 000..d1a6d45
--- /dev/null
+++ b/Zend/tests/ternary.phpt
@@ -0,0 +1,429 @@
+--TEST--
+?: and ternary operators
+--FILE--
+
+ int(1)
+}
+-- 46 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 47 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 48 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 49 --
+string(3) "bar"
+-- 50 --
+string(2) "cd"
+-- 51 --
+string(8) "val_of_b"
+-- 52 --
+string(8) "val_of_b"
+-- 53 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 54 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 55 --
+array(1) {
+ [0]=>
+ int(1)
+}
+-- 56 --
+array(1) {
+ [0]=>
+ int(1)
+}
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index f8fe4ef..221d4f9 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -1469,7 +1469,8 @@ void zend_do_free(znode *op1 TSRMLS_DC) /* {{{ */
&& opline->result.var == op1->u.op.var) {
if (opline->opcode == ZEND_FETCH_R ||
opline->opcode == ZEND_FETCH_DIM_R ||
- opline->opcode == ZEND_FETCH_OBJ_R) {
+ opline->opcode == ZEND_FETCH_OBJ_R ||
+ opline->opcode == ZEND_QM_ASSIGN_VAR) {
/* It's very rare and useless case. It's better to use
additional FREE opcode and simplify the FETCH handlers
their selves */
@@ -6308,8 +6309,13 @@ void zend_do_jmp_set(const znode *value, znode *jmp_token, znode *colon_token TS
int op_number = get_next_op_number(CG(active_op_array));
zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC);
- opline->opcode = ZEND_JMP_SET;
- opline->result_type = IS_TMP_VAR;
+ if (value->op_type == IS_VAR || value->op_type == IS_CV) {
+ opline->opcode = ZEND_JMP_SET_VAR;
+ opline->result_type = IS_VAR;
+ } else {
+ opline->opcode = ZEND_JMP_SET;
+ opline->result_type = IS_TMP_VAR;
+ }
opline->result.var = get_temporary_variable(CG(active_op_array));
SET_NODE(opline->op1, value);
SET_UNUSED(opline->op2);
@@ -6326,9 +6332,20 @@ void zend_do_jmp_set_else(znode *result, const znode *false_value, const znode *
{
zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC);
- opline->opcode = ZEND_QM_ASSIGN;
- opline->extended_value = 0;
SET_NODE(opline->result, colon_token);
+ if (colon_token->op_type == IS_TMP_VAR) {
+ if (false_value->op
[PHP-DEV] Re: Ternary operator improvements
Hi Dmitry,
Le Monday 11 July 2011 09:49:27, Dmitry Stogov a écrit :
> Hi Arnaud,
>
> Is the main advantage of the patch - elimination of zval copy
> constructor for IS_VAR and IS_CV operands?
Yes.
Currently the ternary operator does a copy of its second and third operands.
This is costly when the operand is an array for example (the array has to be
copied, and all its elements addref'ed).
The patch avoids this when the operand is a variable (VAR/CV).
$b = range(1, 1);
for ($i = 0; $i < 10; ++$i) {
true ? $b : $c
}
This code runs in more than a minute currently, because $b is copied on each
iteration.
With the patch, it runs in less than a hundredth or a second.
>
> Does it improve speed only for VAR/CV array operands?
Yes.
For TMP operands this is already fast (no copy). For CONST operands there is
still a copy.
>
> Will the following script require an extra zval ealloc/efree with the
> patch?
>
> function foo($x, $y) {
> $x ? $y : "default";
> }
>
> foo(false, "something");
> ?>
Yes, when only one of the operands is a TMP/CONST, this requires an extra zval
alloc when the TMP/CONST is returned.
This does not happen when both operands and TMP/CONST, or when no operand is
TMP/CONST.
I believe that the cost of this alloc is cancelled by the gains on the other
operand (assuming that all ternary operators with a VAR/CV and a CONST/TMP
operands in a program will evenly return the VAR/CV or the CONST/TMP).
>
> For now the patch still looks a bit messing to me. Also, I would
> probably prefer to use new opcodes ZEND_QM_ASSIGN_VAR and ZEND_JMP_SET_VAR.
Ok, I'll change the patch to use new opcodes instead
>
> I'll try to think a bit more about it later today.
Thanks
>
> Thanks. Dmitry.
>
> On 07/08/2011 06:39 PM, Arnaud Le Blanc wrote:
> > Hi,
> >
> > Thanks for your answer.
> >
> >> Probably it would be better to support both TMP/VAR result. Or even use
> >
> > different opcodes.
> >
> > Yes. I've changed the patch so that it still uses IS_TMP_VAR when both
> > operands are IS_TMP_VAR or IS_CONST.
> >
> > I've also added some benchmarks in micro_benchmark.php, and a test.phpt
> > that tests all combinations of (true|false) ? (const|tmp|var|cv) :
> > (const|tmp|var| cv) (and also for the ?: operator).
> >
> > The test found a memory leak in the current trunk in the following code:
> >
> > true ? $a : $b; the result of $b is marked UNUSED, but not the result of
> > $a. I've modified zend_do_free() to add a ZEND_FREE op after the whole
> > expression instead.
> >
> > Best regards,
> >
> > Arnaud
> >
> > Le Friday 8 July 2011 07:51:22, vous avez écrit :
> >> Hi Arnaud,
> >>
> >> I haven't test your patch yet, but I got the main idea.
> >> I'll need to carefully analyze it. Probably it would be better to
> >> support both TMP/VAR result. Or even use different opcodes.
> >>
> >> Thanks. Dmitry.
> >>
> >> On 07/07/2011 10:45 PM, Arnaud Le Blanc wrote:
> >>> Hi Dmitry,
> >>>
> >>> The ternary operator always deep-copies its second or third operand,
> >>> and it is very slow compared to an if/else when the operand is an
> >>> array for example.
> >>>
> >>> I tried to improve this by avoiding copies when possible. I ran the
> >>> test suite and seen no problem, but I'm not sure if this is correct or
> >>> not; could you please review the attached patch ?
> >>>
> >>> The following script shows the improvements:
> >>>
> >>> $a = range(1, 1);
> >>> for($i = 0; $i< 10; ++$i) {
> >>>
> >>> $a = true ? $a : $a;
> >>>
> >>> }
> >>>
> >>> Without the patch, this runs in more than a minute. With the patch,
> >>> this runs in less than 0.01 second.
> >>>
> >>> I copied most of the code from the ZEND_RETURN handler; I'm not sure
> >>> why IS_CONST are copied as well ? (can't we prevent the IS_CONST from
> >>> being modified by incrementing its refcount ?)
> >>>
> >>> Best regards,
> >>>
> >>> Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Experiments with a threading library for Zend: spawning a new executor
Hi, Le mardi 18 janvier 2011 à 23:36 +0100, Hannes Landeholm a écrit : > Just a simple threading > implementation with a strictly defined way to IPC would be very helpful. If you just want to throw some executors and pass messages between them you can already fork processes with pcntl [1] and pass messages in a variety of ways with [2][3][4], or just plain files :-) This is often enough for speeding up batch scripts or creating some simple servers. [1] http://php.net/manual/en/function.pcntl-fork.php [2] http://php.net/manual/en/book.shmop.php [3] http://php.net/manual/en/book.sem.php [4] http://php.net/stream_socket_pair Best Regards, > > On 18 January 2011 23:10, Stas Malyshev wrote: > > > Hi! > > > > > > Sorry, but that's my topic, and the most well know interpreters that > >> 'pulled off' threading with shared data are for Java. The interpreter > >> > > > > Given to what complications Java programmers should go to make their > > threaded code work, I have a lot of doubt that 95% of PHP users would be > > able to write correct threaded programs. Reasoning about threaded programs > > is very hard, and IMHO putting it into the beginners language would be a > > mistake. > > > > -- > > Stanislav Malyshev, Software Architect > > SugarCRM: http://www.sugarcrm.com/ > > (408)454-6900 ext. 227 > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Upload progress in sessions
Le vendredi 07 mai 2010 à 18:57 +0200, Hannes Magnusson a écrit : > 2010/5/4 Jaroslav Hanslik : > > Hi, > > > > upload progress in session is implemented in old trunk. Is there a change to > > apply the patch also to the new trunk? > > > > http://wiki.php.net/rfc/session_upload_progress > > > > Seems like a good idea to me.. Hi, If RMs agree I will port it to the new trunk. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Implementing fdopen in PHP
Le dimanche 14 mars 2010 à 17:38 +1100, Dennis Hotson a écrit : > Hi all, > > It's my first post, so go easy on me. :-) > > I'm trying to implement PHP support for github's erlang RPC server called > ernie. > So basically I've been porting the following ruby code to PHP: > http://github.com/mojombo/ernie/blob/master/lib/ernie.rb > > The problem I'm having is on line 127-128: > input = IO.new(3) > > I believe this is equivalent to fdopen() in C, but there doesn't appear to > be any way to do this in PHP. > > So basically, I'm a bit stuck and looking for advice on how to proceed. > Should I implement this in core PHP or as an extension? What's the best way > to get a function like this into PHP? Hi, I guess that this can go to some future PHP release, and in a small extension if you want to have it in current / older versions too. You can implement it with php_stream_fopen_from_fd(). Regards, -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Stream: Cast and Seek
Le samedi 05 décembre 2009 à 18:00 +0100, Samuel ROZE a écrit : > Thanks for you reply ! :-) > I think that my cast isn't very good because I've: > > Warning: stream_select(): You MUST recompile PHP with a larger value of > FD_SETSIZE. > It is set to 1024, but you have descriptors numbered at least as high as > 1728763902. > --enable-fd-setsize=1728763904 is recommended, but you may want to set it > to equal the maximum number of open files supported by your system, > in order to avoid seeing this error again at a later date. in ... Check if stream->abstract is a php_ssh2_session_data (looks like it may be a _channel_data instead). > > That my cast code (patch on PHP 5.3 branche): > http://www.d-sites.com/wp-content/uploads/2009/12/cast_ssh2_stream.patch > > Is there something wrong ? > > Regards, > Samuel. > > Le samedi 05 décembre 2009 à 13:40 +0100, Arnaud Le Blanc a écrit : > > Hi, > > > > Le samedi 05 décembre 2009 à 00:01 +0100, Samuel ROZE a écrit : > > > Hello, > > > > > > I'm working on the use of a PHP SSH2 Stream returned by ssh2_shell > > > function with stream_select() function. Within the PHP code, before > > > being used into the select() C function, a stream have to be "casted"... > > > The problem is that I don't really know what is it and how it generally > > > works. > > > > > > So, can someone explain to me what means "cast" and "seek" streams ? > > > > > > (In fact, I have to cast a php_ssh2_channel_stream) > > > > Here you need to return the underlaying socket on which the ssh session > > is established, so that the OS's select() can work on it. Look at > > php_openssl_sockop_cast for an example. > > > > Regards, > > > > Arnaud > > > > > > > > Thanks a lot. > > > Regards, > > > Samuel ROZE. > > > > > > > > > > > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Stream: Cast and Seek
Hi, Le samedi 05 décembre 2009 à 00:01 +0100, Samuel ROZE a écrit : > Hello, > > I'm working on the use of a PHP SSH2 Stream returned by ssh2_shell > function with stream_select() function. Within the PHP code, before > being used into the select() C function, a stream have to be "casted"... > The problem is that I don't really know what is it and how it generally > works. > > So, can someone explain to me what means "cast" and "seek" streams ? > > (In fact, I have to cast a php_ssh2_channel_stream) Here you need to return the underlaying socket on which the ssh session is established, so that the OS's select() can work on it. Look at php_openssl_sockop_cast for an example. Regards, Arnaud > > Thanks a lot. > Regards, > Samuel ROZE. > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: extending ignore_errors inside http_fopen_wrappers.c
Yes, the fix requires "ignore_errors", which is new in 5.3. The bug can't be fixed in 5.2 without breaking compatibility. Regards, Arnaud On Sun, 2009-05-17 at 02:02 +0300, Jani Taskinen wrote: > Last 7 commits or so, none went to PHP_5_2. Intentionally? > We usually fix bugs in the branch they're reported for also.. > > --Jani > > > Arnaud Le Blanc kirjoitti: > > I commited your change as it fixes a bug, thanks for the patch :) > > > > I also followed your idea of a server to test http streams (ftp > > extension does this, too) and wrote a test for this bug. Other http > > tests are welcome if you want to write some. > > > > Regards, > > > > Arnaud > > > > On Sat, 2009-05-16 at 16:42 +0800, Tjerk Anne Meesters wrote: > >> On a related note, I've been working on a phpt to verify the behaviour > >> during which I've noticed a lack of test cases for networked functions > >> in general. > >> > >> The solution I have in mind depends on pcntl_fork(), which practically > >> takes out Windows test targets. It essentially forks out a child to > >> perform the test while the parent starts a socket_server. > >> > >> I suppose it would be nice if the test framework could somehow be > >> extended to pose as a webserver to verify typical http behaviour > >> inside stream wrappers =D > >> > >> This can't be the first time that this has been a point of discussion; > >> my apologies if this has been outright rejected before ;-) > >> > >> On 5/16/09, Arnaud Le Blanc wrote: > >>> On Fri, 2009-05-15 at 11:49 -0400, Ilia Alshanetsky wrote: > >>>> the patch looks good. > >>> Same here. Lukas, Johannes, can this be commited to 5.3 ? > >>> > >>> Regards, > >>> > >>> Arnaud > >>> > >>> > >>>> > >>>> Ilia Alshanetsky > >>>> > >>>> > >>>> > >>>> > >>>> On 15-May-09, at 11:36 AM, Tjerk Anne Meesters wrote: > >>>> > >>>>> Hi Arnaud, > >>>>> > >>>>> Thanks for the tip, please find patch attached. > >>>>> > >>>>> > >>>>> Best, > >>>>> Tjerk > >>>>> > >>>>> On Fri, May 15, 2009 at 10:58 PM, Arnaud Le Blanc > >>>>> wrote: > >>>>>> Hi, > >>>>>> > >>>>>> On Fri, 2009-05-15 at 22:16 +0800, Tjerk Anne Meesters wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> I've been extending the pecl/oauth code to work with php stream > >>>>>>> wrappers in cases whereby curl is not enabled, but I've hit a snag. > >>>>>>> > >>>>>>> The documentation states that enabling the "ignore_errors" flag will > >>>>>>> fetch the content even on failure status codes. At the same time, > >>>>>>> setting "max_redirects" to <= 1 indicates that no redirects will be > >>>>>>> followed. > >>>>>>> > >>>>>>> It does not state that setting "max_redirects" <= 1 would actually > >>>>>>> trigger an error, though in the code it returns (php_stream *)NULL > >>>>>>> making it impossible to even get the headers out of the wrapperdata > >>>>>>> hash unless STREAM_ONLY_GET_HEADERS is used. > >>>>>>> > >>>>>>> So, either setting "max_redirects" <= 1 should not throw NULL back > >>>>>>> into the caller code or "ignore_error" behaviour should be > >>>>>>> extended to > >>>>>>> prevent that from happening. > >>>>>>> > >>>>>>> My attached patch favours to fix the latter, since the > >>>>>>> "ignore_errors" > >>>>>>> is a relatively new flag. Hope it will be accepted for the 5.3 > >>>>>>> release. > >>>>>> max_redirects's purpose is to avoid infinite loops, etc. Stream > >>>>>> functions are expected to return an error in case the limit is > >>>>>> exceeded. > >>>>>> If they do not, scripts will get an empty body without noticing the > >>>>>> error (changing this may break existing scripts). > >>>>>> > >>>>>> ignore_errors is a new flag, it forces scripts to check HTTP status > >>>>>> code > >>>>>> (so that scripts will notice non-200 ones) and changing it to ignore > >>>>>> redirects when max_redirects is exceeded seems to be a good solution. > >>>>>> > >>>>>>> (not sure whether attachments will be allowed ... give me a ping > >>>>>>> if it > >>>>>>> doesn't make it through) > >>>>>> The list accepts only text/plain attachments (try renaming your patch > >>>>>> to .txt). > >>>>>> > >>>>>> > >>>>>> Regards, > >>>>>> > >>>>>> Arnaud > >>>>>> > >>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> -- > >>>>> Tjerk > >>>>> -- > >>>>> PHP Internals - PHP Runtime Development Mailing List > >>>>> To unsubscribe, visit: http://www.php.net/unsub.php > >>> > >> > > > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: extending ignore_errors inside http_fopen_wrappers.c
I commited your change as it fixes a bug, thanks for the patch :) I also followed your idea of a server to test http streams (ftp extension does this, too) and wrote a test for this bug. Other http tests are welcome if you want to write some. Regards, Arnaud On Sat, 2009-05-16 at 16:42 +0800, Tjerk Anne Meesters wrote: > On a related note, I've been working on a phpt to verify the behaviour > during which I've noticed a lack of test cases for networked functions > in general. > > The solution I have in mind depends on pcntl_fork(), which practically > takes out Windows test targets. It essentially forks out a child to > perform the test while the parent starts a socket_server. > > I suppose it would be nice if the test framework could somehow be > extended to pose as a webserver to verify typical http behaviour > inside stream wrappers =D > > This can't be the first time that this has been a point of discussion; > my apologies if this has been outright rejected before ;-) > > On 5/16/09, Arnaud Le Blanc wrote: > > > > On Fri, 2009-05-15 at 11:49 -0400, Ilia Alshanetsky wrote: > >> the patch looks good. > > > > Same here. Lukas, Johannes, can this be commited to 5.3 ? > > > > Regards, > > > > Arnaud > > > > > >> > >> > >> Ilia Alshanetsky > >> > >> > >> > >> > >> On 15-May-09, at 11:36 AM, Tjerk Anne Meesters wrote: > >> > >> > Hi Arnaud, > >> > > >> > Thanks for the tip, please find patch attached. > >> > > >> > > >> > Best, > >> > Tjerk > >> > > >> > On Fri, May 15, 2009 at 10:58 PM, Arnaud Le Blanc > >> > wrote: > >> >> Hi, > >> >> > >> >> On Fri, 2009-05-15 at 22:16 +0800, Tjerk Anne Meesters wrote: > >> >>> Hi, > >> >>> > >> >>> I've been extending the pecl/oauth code to work with php stream > >> >>> wrappers in cases whereby curl is not enabled, but I've hit a snag. > >> >>> > >> >>> The documentation states that enabling the "ignore_errors" flag will > >> >>> fetch the content even on failure status codes. At the same time, > >> >>> setting "max_redirects" to <= 1 indicates that no redirects will be > >> >>> followed. > >> >>> > >> >>> It does not state that setting "max_redirects" <= 1 would actually > >> >>> trigger an error, though in the code it returns (php_stream *)NULL > >> >>> making it impossible to even get the headers out of the wrapperdata > >> >>> hash unless STREAM_ONLY_GET_HEADERS is used. > >> >>> > >> >>> So, either setting "max_redirects" <= 1 should not throw NULL back > >> >>> into the caller code or "ignore_error" behaviour should be > >> >>> extended to > >> >>> prevent that from happening. > >> >>> > >> >>> My attached patch favours to fix the latter, since the > >> >>> "ignore_errors" > >> >>> is a relatively new flag. Hope it will be accepted for the 5.3 > >> >>> release. > >> >> > >> >> max_redirects's purpose is to avoid infinite loops, etc. Stream > >> >> functions are expected to return an error in case the limit is > >> >> exceeded. > >> >> If they do not, scripts will get an empty body without noticing the > >> >> error (changing this may break existing scripts). > >> >> > >> >> ignore_errors is a new flag, it forces scripts to check HTTP status > >> >> code > >> >> (so that scripts will notice non-200 ones) and changing it to ignore > >> >> redirects when max_redirects is exceeded seems to be a good solution. > >> >> > >> >>> > >> >>> (not sure whether attachments will be allowed ... give me a ping > >> >>> if it > >> >>> doesn't make it through) > >> >> > >> >> The list accepts only text/plain attachments (try renaming your patch > >> >> to .txt). > >> >> > >> >> > >> >> Regards, > >> >> > >> >> Arnaud > >> >> > >> >> > >> >> > >> > > >> > > >> > > >> > -- > >> > -- > >> > Tjerk > >> > -- > >> > PHP Internals - PHP Runtime Development Mailing List > >> > To unsubscribe, visit: http://www.php.net/unsub.php > >> > > > > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] extending ignore_errors inside http_fopen_wrappers.c
On Fri, 2009-05-15 at 11:49 -0400, Ilia Alshanetsky wrote: > the patch looks good. Same here. Lukas, Johannes, can this be commited to 5.3 ? Regards, Arnaud > > > Ilia Alshanetsky > > > > > On 15-May-09, at 11:36 AM, Tjerk Anne Meesters wrote: > > > Hi Arnaud, > > > > Thanks for the tip, please find patch attached. > > > > > > Best, > > Tjerk > > > > On Fri, May 15, 2009 at 10:58 PM, Arnaud Le Blanc > > wrote: > >> Hi, > >> > >> On Fri, 2009-05-15 at 22:16 +0800, Tjerk Anne Meesters wrote: > >>> Hi, > >>> > >>> I've been extending the pecl/oauth code to work with php stream > >>> wrappers in cases whereby curl is not enabled, but I've hit a snag. > >>> > >>> The documentation states that enabling the "ignore_errors" flag will > >>> fetch the content even on failure status codes. At the same time, > >>> setting "max_redirects" to <= 1 indicates that no redirects will be > >>> followed. > >>> > >>> It does not state that setting "max_redirects" <= 1 would actually > >>> trigger an error, though in the code it returns (php_stream *)NULL > >>> making it impossible to even get the headers out of the wrapperdata > >>> hash unless STREAM_ONLY_GET_HEADERS is used. > >>> > >>> So, either setting "max_redirects" <= 1 should not throw NULL back > >>> into the caller code or "ignore_error" behaviour should be > >>> extended to > >>> prevent that from happening. > >>> > >>> My attached patch favours to fix the latter, since the > >>> "ignore_errors" > >>> is a relatively new flag. Hope it will be accepted for the 5.3 > >>> release. > >> > >> max_redirects's purpose is to avoid infinite loops, etc. Stream > >> functions are expected to return an error in case the limit is > >> exceeded. > >> If they do not, scripts will get an empty body without noticing the > >> error (changing this may break existing scripts). > >> > >> ignore_errors is a new flag, it forces scripts to check HTTP status > >> code > >> (so that scripts will notice non-200 ones) and changing it to ignore > >> redirects when max_redirects is exceeded seems to be a good solution. > >> > >>> > >>> (not sure whether attachments will be allowed ... give me a ping > >>> if it > >>> doesn't make it through) > >> > >> The list accepts only text/plain attachments (try renaming your patch > >> to .txt). > >> > >> > >> Regards, > >> > >> Arnaud > >> > >> > >> > > > > > > > > -- > > -- > > Tjerk > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] extending ignore_errors inside http_fopen_wrappers.c
Hi, On Fri, 2009-05-15 at 22:16 +0800, Tjerk Anne Meesters wrote: > Hi, > > I've been extending the pecl/oauth code to work with php stream > wrappers in cases whereby curl is not enabled, but I've hit a snag. > > The documentation states that enabling the "ignore_errors" flag will > fetch the content even on failure status codes. At the same time, > setting "max_redirects" to <= 1 indicates that no redirects will be > followed. > > It does not state that setting "max_redirects" <= 1 would actually > trigger an error, though in the code it returns (php_stream *)NULL > making it impossible to even get the headers out of the wrapperdata > hash unless STREAM_ONLY_GET_HEADERS is used. > > So, either setting "max_redirects" <= 1 should not throw NULL back > into the caller code or "ignore_error" behaviour should be extended to > prevent that from happening. > > My attached patch favours to fix the latter, since the "ignore_errors" > is a relatively new flag. Hope it will be accepted for the 5.3 > release. max_redirects's purpose is to avoid infinite loops, etc. Stream functions are expected to return an error in case the limit is exceeded. If they do not, scripts will get an empty body without noticing the error (changing this may break existing scripts). ignore_errors is a new flag, it forces scripts to check HTTP status code (so that scripts will notice non-200 ones) and changing it to ignore redirects when max_redirects is exceeded seems to be a good solution. > > (not sure whether attachments will be allowed ... give me a ping if it > doesn't make it through) The list accepts only text/plain attachments (try renaming your patch to .txt). Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Fix for bug #45540 not in PHP_5_2
Hi, On Thu, 2009-05-14 at 17:14 +0300, Jani Taskinen wrote: > Hi, > > Why wasn't this fix merged to PHP_5_2? It's clearly a bug and it really > should > be fixed in the stable branch as well.. The fix changes a parameter of php_stream_url_wrap_http_ex(), I guess that I thought that it was unsuitable for 5.2. Or maybe at this time I thought the 5.2 branch was closed. The parameter change may not break code using it, I will merge this. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Bug 47468: enabling readline/libedit/pcntl/ncurses with --enable-embed
On Sat, 2009-05-09 at 03:45 +0300, Jani Taskinen wrote: > Arnaud Le Blanc kirjoitti: > > On Fri, 2009-05-08 at 16:12 +0100, Paul Biggar wrote: > >> Hi Arnaud, > >> > >> Thanks for looking at this. > >> > >> On Fri, May 8, 2009 at 2:11 PM, Arnaud Le Blanc wrote: > >>> Does the following patch works for you ? (use ./buildconf after > >>> applying, then reconfigure). It does the same thing as yours, but moves > >>> the decision of allowing "cli" extensions to SAPI's config.m4. > >> I tested this on CVS 5.3 (with the configure command: ./configure > >> --with-readline --enable-embed --enable-maintainer-zts > >> --enable-debug). It applies cleanly, and I checked the readline > >> symbols were present. I have also forwarded it to two of phc's users > >> who reported the bug. I expect they'll be back to me within a day or > >> two, if you prefer to wait for further confirmation. > >> > >> FWIW, I like the new patch better than my hack. However, I don't > >> understand the how it works from the comment (that bit of acinclude is > >> very shoddily documented in general). > > > > The build system allows to build two SAPIs at a time: the CLI and a SAPI > > of choice. Some extensions are irrelevant for webserver SAPIs, so they > > mark themselves as CLI-only extensions. This allows one to enable these > > extensions in the CLI without enabling them in the server SAPI. This > > patch allows a SAPI to mark itself as allowing those extensions. > > I really need to sleep before I read this again, but with a quick look at the > patch I couldn't figure what you're fixing and where. :) Reproduce code: "./configure --with-readline --enable-embed && make" Expected result: libphp5.so with readline extension builtin Actual result: libphp5.so without readline extension Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Bug 47468: enabling readline/libedit/pcntl/ncurses with --enable-embed
On Fri, 2009-05-08 at 16:12 +0100, Paul Biggar wrote: > Hi Arnaud, > > Thanks for looking at this. > > On Fri, May 8, 2009 at 2:11 PM, Arnaud Le Blanc wrote: > > Does the following patch works for you ? (use ./buildconf after > > applying, then reconfigure). It does the same thing as yours, but moves > > the decision of allowing "cli" extensions to SAPI's config.m4. > > I tested this on CVS 5.3 (with the configure command: ./configure > --with-readline --enable-embed --enable-maintainer-zts > --enable-debug). It applies cleanly, and I checked the readline > symbols were present. I have also forwarded it to two of phc's users > who reported the bug. I expect they'll be back to me within a day or > two, if you prefer to wait for further confirmation. > > FWIW, I like the new patch better than my hack. However, I don't > understand the how it works from the comment (that bit of acinclude is > very shoddily documented in general). The build system allows to build two SAPIs at a time: the CLI and a SAPI of choice. Some extensions are irrelevant for webserver SAPIs, so they mark themselves as CLI-only extensions. This allows one to enable these extensions in the CLI without enabling them in the server SAPI. This patch allows a SAPI to mark itself as allowing those extensions. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Bug 47468: enabling readline/libedit/pcntl/ncurses with --enable-embed
Hi, On Wed, 2009-05-06 at 20:42 +0100, Paul Biggar wrote: > Hi folks, > > Could I get someone to look at http://bugs.php.net/bug.php?id=47468?. > It includes a patch which is confirmed to fix the problem. Does the following patch works for you ? (use ./buildconf after applying, then reconfigure). It does the same thing as yours, but moves the decision of allowing "cli" extensions to SAPI's config.m4. Lukas, Johannes, any objection ? Regards, Arnaud Index: acinclude.m4 === RCS file: /repository/php-src/acinclude.m4,v retrieving revision 1.332.2.14.2.26.2.12 diff -u -p -r1.332.2.14.2.26.2.12 acinclude.m4 --- acinclude.m43 Dec 2008 19:53:45 - 1.332.2.14.2.26.2.12 +++ acinclude.m48 May 2009 12:38:11 - @@ -877,15 +877,17 @@ EOF ]) dnl -dnl PHP_SELECT_SAPI(name, type[, sources [, extra-cflags [, build-target]]]) +dnl PHP_SELECT_SAPI(name, type[, sources [, extra-cflags [, build-target [, allow-static-exts) dnl dnl Selects the SAPI name and type (static, shared, programm) dnl and optionally also the source-files for the SAPI-specific dnl objects. dnl +dnl allow-static-exts: Whether SAPI allows any extension to be built statically ([yes], all) +dnl AC_DEFUN([PHP_SELECT_SAPI],[ PHP_SAPI=$1 - + case "$2" in static[)] PHP_BUILD_STATIC;; shared[)] PHP_BUILD_SHARED;; @@ -894,6 +896,12 @@ AC_DEFUN([PHP_SELECT_SAPI],[ esac ifelse($3,,,[PHP_ADD_SOURCES([sapi/$1],[$3],[$4],[sapi])]) + + case "$6" in + [yes|all)] PHP_SAPI_ALLOWS_STATIC_EXTS="$6";; + ["")] PHP_SAPI_ALLOWS_STATIC_EXTS=yes;; + esac + ]) dnl deprecated @@ -968,7 +976,7 @@ dnl if test "$3" != "shared" && test "$3" != "yes" && test "$4" = "cli"; then dnl -- CLI static module [PHP_]translit($1,a-z_-,A-Z__)[_SHARED]=no -if test "$PHP_SAPI" = "cgi"; then +if test "$PHP_SAPI_ALLOWS_STATIC_EXTS" = "all"; then PHP_ADD_SOURCES(PHP_EXT_DIR($1),$2,$ac_extra,) EXT_STATIC="$EXT_STATIC $1" else Index: sapi/cgi/config9.m4 === RCS file: /repository/php-src/sapi/cgi/config9.m4,v retrieving revision 1.17.2.2.2.6.2.2 diff -u -p -r1.17.2.2.2.6.2.2 config9.m4 --- sapi/cgi/config9.m4 1 Oct 2007 12:40:54 - 1.17.2.2.2.6.2.2 +++ sapi/cgi/config9.m4 8 May 2009 12:38:14 - @@ -54,7 +54,7 @@ if test "$PHP_SAPI" = "default"; then dnl Set install target and select SAPI INSTALL_IT="@echo \"Installing PHP CGI binary: \$(INSTALL_ROOT)\$(bindir)/\"; \$(INSTALL) -m 0755 \$(SAPI_CGI_PATH) \$(INSTALL_ROOT)\$(bindir)/\$(program_prefix)php-cgi\$(program_suffix)\$(EXEEXT)" -PHP_SELECT_SAPI(cgi, program, cgi_main.c fastcgi.c,, '$(SAPI_CGI_PATH)') +PHP_SELECT_SAPI(cgi, program, cgi_main.c fastcgi.c,, '$(SAPI_CGI_PATH)', all) case $host_alias in *aix*) Index: sapi/embed/config.m4 === RCS file: /repository/php-src/sapi/embed/config.m4,v retrieving revision 1.9.4.2 diff -u -p -r1.9.4.2 config.m4 --- sapi/embed/config.m411 Jul 2007 23:20:36 - 1.9.4.2 +++ sapi/embed/config.m48 May 2009 12:38:14 - @@ -23,7 +23,7 @@ if test "$PHP_EMBED" != "no"; then ;; esac if test "$PHP_EMBED_TYPE" != "no"; then -PHP_SELECT_SAPI(embed, $PHP_EMBED_TYPE, php_embed.c) +PHP_SELECT_SAPI(embed, $PHP_EMBED_TYPE, php_embed.c, , , all) PHP_INSTALL_HEADERS([sapi/embed/php_embed.h]) fi AC_MSG_RESULT([$PHP_EMBED_TYPE]) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PHP 5.3.0RC2
On Mon, May 4, 2009 at 5:29 PM, Lukas Kahwe Smith wrote: > until then we need to > make sure we can get that re2c bug fixed and a new release out. this might > require reducing the bus factor on re2c. A good starting point for re2c internals is the following paper: http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.27.1624 It looks like feasible to add the EOF feature to re2c (which means switching to states depending of their depth (to ensure to not read ahead), and becoming ungreedy on regexps like "foo"+ when their is no additional chars (to unsure to match a token without having to see an other char next to it)). Is anyone working on it already ? (or any thought on this ?) Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Scanner "diet" with fixes, etc.
On Mon, May 4, 2009 at 5:51 PM, shire wrote:
> Arnaud Le Blanc wrote:
>>
>> Hi,
>> On Mon, May 4, 2009 at 9:36 AM, shire wrote:
>>>
>>> Regarding the ZEND_MMAP_AHEAD issue and the temp. fix that Dmitry put in
>>> we
>>> need to find a solution to that, perhaps I can play with that this week
>>> too
>>> as I think I'm seeing some related issues in my testing of 5.3.
>>> Essentially
>>> we abuse ZEND_MMAP_AHEAD by adding it to the file size and passing it to
>>> the
>>> mmap call which isn't at all valid and only really works up to PAGESIZE.
>>> We
>>> could possibly use YYFILL to re-allocate more space as necessary past the
>>> end of file to fix this.
>>
>> I was thinking of doing something like that with YYFILL too. However
>> there is a bunch of pointers to take in to account and to update (e.g.
>> yy_marker, yy_text, etc).
>>
>
> Yeah, I'm pretty sure that's how most of the example re2c code is setup:
>
> #define YYFILL(n) {cursor = fill(s, cursor);}
>
> uchar *fill(Scanner *s, uchar *cursor){
> if(!s->eof){
> unint cnt = s->lim - s->tok;
> uchar *buf = malloc((cnt + 1)*sizeof(uchar));
> memcpy(buf, s->tok, cnt);
> cursor = &buf[cursor - s->tok];
> s->pos = &buf[s->pos - s->tok];
> s->ptr = &buf[s->ptr - s->tok];
> s->lim = &buf[cnt];
> s->eof = s->lim; *(s->eof)++ = '\n';
> s->tok = buf;
> }
> return cursor;
> }
>
>
>
> -shire
>
This is what I seen too, but this is not always applicable. The
scanner have code that refers to yy_text, yy_start, yy_cursor,
yy_marker, etc. All those pointers point to the original buffer and
must be updated by fill(). At each point in time the scanner may
rollback to yy_marker or a rule may want to fetch yy_text or yy_start
at any time. So the buffer must be large enough to contain all data
from min(all_of_them) to max(all_of_them). That makes things a little
complicated and potentially less efficient than a big buffer for the
whole file.
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Scanner "diet" with fixes, etc.
Hi, On Mon, May 4, 2009 at 9:36 AM, shire wrote: > Regarding the ZEND_MMAP_AHEAD issue and the temp. fix that Dmitry put in we > need to find a solution to that, perhaps I can play with that this week too > as I think I'm seeing some related issues in my testing of 5.3. Essentially > we abuse ZEND_MMAP_AHEAD by adding it to the file size and passing it to the > mmap call which isn't at all valid and only really works up to PAGESIZE. We > could possibly use YYFILL to re-allocate more space as necessary past the > end of file to fix this. I was thinking of doing something like that with YYFILL too. However there is a bunch of pointers to take in to account and to update (e.g. yy_marker, yy_text, etc). There is mmap(MAP_FIXED) as an other temporary solution. The idea is to map the entire file rounded up to a whole page size, and to map the last (already mapped) page to anonymous memory using MAP_FIXED. In the end we get a readable contiguous memory region with the file data and ZEND_MMAP_AHEAD. This should work on many systems as long as the requested address is already part of the process's address space (which is always the case here). When this fails we can fallback to malloc/read. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Support for "Transfer-Encoding: chunked" in http stream wrapper
Hi, On Tue, 2009-04-14 at 18:59 +0400, Dmitry Stogov wrote: > Some PHP applications which check for Transfer-Encoding HTTP header and > perform manual decoding might be broken. What about not adding the Transfer-Encoding header to wrapper_data ? (not making it visible so that scripts would not break). Also, re-enabling the filters list in stream_get_meta_data() so that one can know that the dechunk filter has been added. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] un-deprecating ticks ?
On Thu, 2009-03-26 at 08:51 +0100, Lukas Kahwe Smith wrote: > On 26.03.2009, at 01:30, Arnaud Le Blanc wrote: > > > On Wed, 2009-03-25 at 20:05 +0100, Lukas Kahwe Smith wrote: > >> On 19.03.2009, at 14:32, Arnaud Le Blanc wrote: > >> > >>> Hi, > >>> > >>> After having seen some complaints about ticks being deprecated I'm > >>> wondering if they could be un-deprecated for now. > >>> > >>> Ticks are used by the pcntl extension to call signal handlers when > >>> signals are triggered. I added some functions as an alternative, but > >>> this does not covers all use cases (and forces a code change). > >>> > >>> When searching bug reports about ticks, one can feel the ticks to be > >>> broken (and this is why they have been deprecated). However, looking > >>> in > >>> depth at these bugs and viewing what caused them and how they have > >>> been > >>> fixed does not show really bad things about ticks. > >>> > >>> Actually one thing is broken (and is marked as such in the > >>> documentation), tick functions do not work in ZTS, this looks > >>> fixable. > >>> > >>> Any thoughts on removing the deprecation warning for now ? (at least > >>> until a replacement is found). > >>> > >>> Sorry for posting this so close to the freeze. > >> > >> > >> So what is it going to be? > >> I remember everybody was happy when it was deprecated. > > > > The only thread I found about that is here: > > http://marc.info/?l=php-internals&m=121442930703916&w=2 > > > >> Indeed the only open ticket is #47198, which is the doc bug. Or did > >> we > >> close tons of ticks bugs because it was deprecated? > > > > I've seen one bug marked as wont fix for this reason. I've searched > > for > > "ticks" and looked at the last few pages of bugs, many was really > > bogus, > > or documentation issues. Some was related to register_tick_function() > > and are fixed. I seen nothing really bad or related to the engine. > > > > The ZTS issue looks fixable, this is a crash when using > > register_tick_function() due to the list of functions not being > > initialized > > in the threads. > > > It seems to me like at this point its your call. You are the one that > has dug into this. If you feel you can fix things then I guess its > your call to undeprecate ticks. As for the bogus bugs, this maybe > indicate that we need to do some more work on the documentation. Could > you look into this as well? Overall we should do our dearest to > prevent that even stupid users can crash PHP and if something can too > easily be made to still crash PHP, maybe its something we shouldnt have. > Ok, so I reverted the deprecation warning and fixed the ZTS issue. I will take a look at the documentation. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] un-deprecating ticks ?
On Wed, 2009-03-25 at 20:05 +0100, Lukas Kahwe Smith wrote: > On 19.03.2009, at 14:32, Arnaud Le Blanc wrote: > > > Hi, > > > > After having seen some complaints about ticks being deprecated I'm > > wondering if they could be un-deprecated for now. > > > > Ticks are used by the pcntl extension to call signal handlers when > > signals are triggered. I added some functions as an alternative, but > > this does not covers all use cases (and forces a code change). > > > > When searching bug reports about ticks, one can feel the ticks to be > > broken (and this is why they have been deprecated). However, looking > > in > > depth at these bugs and viewing what caused them and how they have > > been > > fixed does not show really bad things about ticks. > > > > Actually one thing is broken (and is marked as such in the > > documentation), tick functions do not work in ZTS, this looks fixable. > > > > Any thoughts on removing the deprecation warning for now ? (at least > > until a replacement is found). > > > > Sorry for posting this so close to the freeze. > > > So what is it going to be? > I remember everybody was happy when it was deprecated. The only thread I found about that is here: http://marc.info/?l=php-internals&m=121442930703916&w=2 > Indeed the only open ticket is #47198, which is the doc bug. Or did we > close tons of ticks bugs because it was deprecated? I've seen one bug marked as wont fix for this reason. I've searched for "ticks" and looked at the last few pages of bugs, many was really bogus, or documentation issues. Some was related to register_tick_function() and are fixed. I seen nothing really bad or related to the engine. The ZTS issue looks fixable, this is a crash when using register_tick_function() due to the list of functions not being initialized in the threads. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] un-deprecating ticks ?
Hi, After having seen some complaints about ticks being deprecated I'm wondering if they could be un-deprecated for now. Ticks are used by the pcntl extension to call signal handlers when signals are triggered. I added some functions as an alternative, but this does not covers all use cases (and forces a code change). When searching bug reports about ticks, one can feel the ticks to be broken (and this is why they have been deprecated). However, looking in depth at these bugs and viewing what caused them and how they have been fixed does not show really bad things about ticks. Actually one thing is broken (and is marked as such in the documentation), tick functions do not work in ZTS, this looks fixable. Any thoughts on removing the deprecation warning for now ? (at least until a replacement is found). Sorry for posting this so close to the freeze. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] 5.3 todos
Hi, On Wed, 2009-02-11 at 18:07 +0100, Lukas Kahwe Smith wrote: > - pcntl_signal needs declare(ticks) which is deprecated since 5.3 I marked this as a documentation issue. This has been discussed when it was decided to deprecate ticks. Although it would be great to keep ticks, at least for use with signals, nobody said to be able to make ticks safer (or crash-free). > http://bugs.php.net/bug.php?id=46171 - stream_bucket_new() The entry reports that it is not possible to create a new bucket in a user space stream filter while the stream is closing. The reason is that the filter method is called inside of php_stream_free() and at this point the stream resource is not valid (and stream_bucket_new() takes the stream resource as argument). I can't see a way to fix this. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Allow unsetting headers previously set usingheader()
Hi,
On Friday 28 November 2008 18:24:38 Uwe Schindler wrote:
> Just one question here:
> When implementing this into NSAPI, I found the following problem:
>
> NSAPI does not directly allows to remove all headers, you can only do this
> step by step. So there are three possibilities to ship around this problem:
>
> a) when SAPI_HEADER_DELETE_ALL is given, in header_handler, do the following
> (using the sapi_header_struct given as last parameter):
>
> zend_llist_apply(sapi_headers->headers, (llist_apply_func_t)
> php_nsapi_remove_header TSRMLS_CC);
>
> with:
> static int php_nsapi_remove_header(sapi_header_struct *sapi_header
> TSRMLS_DC)
> {
> char *header_name, *p;
> nsapi_request_context *rc = (nsapi_request_context
> *)SG(server_context);
>
> header_name = nsapi_strdup(sapi_header->header);
> if (p = strchr(header_name, ':')) *p = 0;
>
> ...
>
> param_free(pblock_remove(header_name, rc->rq->srvhdrs));
> nsapi_free(header_name);
>
> return ZEND_HASH_APPLY_KEEP;
> }
>
> This would remove all headers, set by PHP. Headers embedded by the server
> itself would not be deleted (e.g. "Server:" etc.)
>
> b) Use some "hack" to get rid of all headers from the server's hashtable
> (like apr_table_clear()). This would remove all headers and also some
> important headers set by the server itself (like Server:, Chunked response
> headers, etc). I am not sure if this would be good, in my opinion
> SAPI_HEADER_DELETE_ALL should only remove headers set by PHP itself! What
> does the other SAPI developers think, is it safe to remove apaches default
> headers? I am not sure and tend to say: NO!
>
> c) Completely ignore sapi_header_handler like in CGI and set the headers
> later in sapi_send_headers() using the given struct with zend_llist_apply().
> Then I do not have to take care of adding/removing headers, I set them to
> Sun Webserver shortly before starting the response. Why the difference
> between header_handler and send_headers? Is it ok to remove header_handler
> (like in CGI) and simply feed all headers in send_headers? This would make
> life for SAPI developers easier (also maybe in Apache). What is the idea to
> respond after each header change?
I believe that Apache does sets its headers just before sending them, so when
PHP deletes all headers in Apache's hashtable this does not removes "Server",
"Date", etc. If this is not the case for NSAPI, solution a) seems good, but
also allows to remove "Date" by setting it before ;)
One reason for having header_handler() and send_headers() is for example that
flush() does not sends headers explicitly, but lets the server send them based
on what header_handler() has set.
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] question on how to solve major stream filter design flaw
Hi,
On Saturday 11 October 2008 19:45:09 Gregory Beaver wrote:
> Hi,
>
> I'm grappling with a design flaw I just uncovered in stream filters, and
> need some advice on how best to fix it. The problem exists since the
> introduction of stream filters, and has 3 parts. 2 of them can probably
> be fixed safely in PHP 5.2+, but I think the third may require an
> internal redesign of stream filters, and so would probably have to be
> PHP 5.3+, even though it is a clear bugfix (Ilia, your opinion
> appreciated on this).
>
> The first part of the bug that I encountered is best described here:
> http://bugs.php.net/bug.php?id=46026. However, it is a deeper problem
> than this, as the attempts to cache data is dangerous any time a stream
> filter is attached to a stream. I should also note that the patch in
> this bug contains feature additions that would have to wait for PHP 5.3.
>
> I ran into this problem because I was trying to use stream filters to
> read in a bz2-compressed file within a zip archive in the phar
> extension. This was failing, and I first tracked the problem down to an
> attempt by php_stream_filter_append to read in a bunch of data and cache
> it, which caused more stuff to be passed into the bz2 decompress filter
> than it could handle, making it barf. After fixing this problem, I ran
> into the problem described in the bug above because of
> php_stream_fill_read_buffer doing the same thing when I tried to read
> the data, because I requested it return 176 decompressed bytes, and so
> php_stream_read passed in 176 bytes to the decompress filter. Only 144
> of those bytes were actually bz2-compressed data, and so the filter
> barfed upon trying to decompress the remaining data (same as bug #46026,
> found differently).
>
> You can probably tell from my explanation that this is an
> extraordinarily complex problem. There's 3 inter-related problems here:
>
> 1) bz2 (and zlib) stream filter should stop trying to decompress when it
> reaches the stream end regardless of how many bytes it is told to
> decompress (easy to fix)
> 2) it is never safe to cache read data when a read stream filter is
> appended, as there is no safe way to determine in advance how much of
> the stream can be safely filtered. (would be easy to fix if it weren't
> for #3)
> 3) there is no clear way to request that a certain number of filtered
> bytes be returned from a stream, versus how many unfiltered bytes should
> be passed into the stream. (very hard to fix without design change)
>
> I need some advice on #3 from the original designers of stream filters
> and streams, as well as any experts who have dealt with this kind of
> problem in other contexts. In this situation, should we expect stream
> filters to always stop filtering if they reach the end of valid input?
> Even in this situation, there is potential that less data is available
> than passed in. A clear example would be if we requested only 170
> bytes. 144 of those bytes would be passed in as the complete compressed
> data, and bz2.decompress would decompress all of it to 176 bytes. 170
> of those bytes would be returned from php_stream_read, and 6 would have
> to be placed in a cache for future reads. Thus, there would need to be
> some way of marking the cache as valid because of this logic path:
>
> $a = fopen('blah.zip');
> fseek($a, 132); // fills read buffer with unfiltered data
> stream_filter_append($a, 'bzip2.decompress'); // clears read buffer cache
> $b = fread($a, 170); // fills read buffer cache with 6 bytes
> fseek($a, 3, SEEK_CUR); // this should seek within the filtered data
> read buffer cache
> stream_filter_append($a, 'zlib.inflate');
> ?>
>
> The question is what should happen when we append the second filter
> 'zlib.inflate' to filter the filtered data? If we clear the read buffer
> as we did in the first case, it will result in lost data. So, let's
> assume we preserve the read buffer. Then, if we perform:
>
> $c = fread($a, 7);
> ?>
>
> and assume the remaining 3 bytes expand to 8 bytes, how should the read
> buffer cache be handled? Should the first 3 bytes still be the filtered
> bzip2 decompressed data, and the last 3 replaced with the 8 bytes of
> decompressed zlib data?
>
> Basically, I am wondering if perhaps we need to implement a read buffer
> cache for each stream filter. This could solve our problem, I think.
> The data would be stored like so:
>
> stream: 170 bytes of unfiltered data, and a pointer to byte 145 as the
> next byte for php_stream_read()
> bzip2.decompress filter: 176 bytes of decompressed bzip2 data, and a
> pointer to byte 171 as the next byte for php_stream_read()
> zlib.inflate filter: 8 bytes of decompressed zlib data, and a pointer to
> byte 8 as the next byte for php_stream_read()
>
> This way, we would essentially have a stack of stream data. If the zlib
> filter were then removed, we could "back up" to the bzip2 filter and so
> on. This will allow proper read cache f
Re: [PHP-DEV] [PATCH] Allow unsetting headers previously set usingheader()
Hi,
Committed, thanks Christian :)
apache2handler, apache2filter, apache, apache_hooks, cli and cgi SAPIs have
been updated.
The following SAPIs need to be updated in PHP_5_3 and HEAD:
aolserver, continuity, litespeed, nsapi, caudium, phttpd, roxen. (I'm CC-ing
known maintainers)
More informations on the change can be found in the commit message:
http://news.php.net/php.cvs/54228
Regards,
Arnaud
On Sunday 09 November 2008 22:49:47 Uwe Schindler wrote:
> +1
>
> I have no problem with implementing this for NSAPI after the patch is
> committed to CVS, just keep me informed about this.
>
> -
> Uwe Schindler
> [EMAIL PROTECTED] - http://www.php.net
> NSAPI SAPI developer
> Bremen, Germany
>
> > -Original Message-
> > From: Arnaud LB [mailto:[EMAIL PROTECTED] On Behalf Of Arnaud Le Blanc
> > Sent: Sunday, November 09, 2008 10:02 PM
> > To: internals@lists.php.net
> > Cc: Christian Schmidt
> > Subject: Re: [PHP-DEV] [PATCH] Allow unsetting headers previously set
> > usingheader()
> >
> > On Sunday 09 November 2008 19:51:31 Christian Schmidt wrote:
> > > Stan Vassilev | FM wrote:
> > > > I suggest header_remove('*') or simply header_remove() /no param/
> > > > removes all headers (including the one PHP sets by default), so we can
> > > > start with a clear state.
> > >
> > > I added header_remove('Foo'). header_remove() without arguments removes
> > > all headers (though Apache still adds some headers that you cannot
> > remove).
> > >
> > > I have tested with apache2handler and cgi. I had to change the signature
> > > of SAPI header_handler function and sapi_header_struct, so the other
> > > SAPIs should be updated for this. I am not sure how to test all these?
> > > Creating a testing environment for all those webservers seems like a
> > > huge task.
> > >
> > > I am not comfortable with the size of this patch, given my understanding
> > > of the PHP source code and my general C skills, so I am posting this
> > > patch hoping that somebody will pick it up or help me get it into shape.
> > >
> >
> > It looks good. The signature change is not that bad if it forces all SAPIs
> > to
> > be updated and ensures that PHP behaves the same way with all SAPIs.
> >
> > It is also possible to add something like header_delete_handler() or
> > header_handler_ex() to sapi_module_struct if the signature change is to be
> > avoided.
> >
> > Regards,
> >
> > Arnaud
> >
> >
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: http://www.php.net/unsub.php
>
>
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] quick polls for 5.3
On Wednesday 12 November 2008 20:14:31 Lukas Kahwe Smith wrote:
> Hi,
> 1) ext/mhash in 5.3. ext/hash has all the functions, so the entire BC
> break will be that "if (extension_loaded('mhash'))" will need fixing
> if mhash is removed (answer both)
> I) enable ext/hash by default
+1
> II) remove ext/mhash
-0
>
> 2) deprecate ereg*. ext/ereg is an extension as of PHP 5.3. Since ext/
> ereg is more or less redundant with ext/preg and is likely to not get
> much unicode love for PHP 6, the question is if we should mark it with
> a E_DEPRECATED in PHP 5.3
+1 (and allow the --without-ereg switch)
>
> 3) resource constants (choose one)
> a) Should we deprecate constant resources (mostly used to emulate
> STDIN and friends)
> b) Should we instead just throw an E_STRICT
> c) Document as is
a
>
> 4) keep ext/phar enabled by default in 5.3?
+1
>
> 5) keep ext/sqlite3 enabled by default in 5.3?
+1
>
> 6) enable mysqlnd by default in 5.3? (answer both)
> I) enable mysqlnd by default
-0
> II) also enable ext/mysql, mysqli und pdo_mysql by default since there
> will be no external dependencies in this case
>
> 7) should Output buffering rewrite MFH? this one comes with some
> baggage, we need enough people to actually have a look at how things
> are in HEAD and make it clear that they will be available for bug
> fixing and BC issues resolving. the risk here is obviously that any BC
> issues will be hard to isolate for end users.
+0
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Allow unsetting headers previously set usingheader()
On Sunday 09 November 2008 19:51:31 Christian Schmidt wrote:
> Stan Vassilev | FM wrote:
> > I suggest header_remove('*') or simply header_remove() /no param/
> > removes all headers (including the one PHP sets by default), so we can
> > start with a clear state.
>
> I added header_remove('Foo'). header_remove() without arguments removes
> all headers (though Apache still adds some headers that you cannot remove).
>
> I have tested with apache2handler and cgi. I had to change the signature
> of SAPI header_handler function and sapi_header_struct, so the other
> SAPIs should be updated for this. I am not sure how to test all these?
> Creating a testing environment for all those webservers seems like a
> huge task.
>
> I am not comfortable with the size of this patch, given my understanding
> of the PHP source code and my general C skills, so I am posting this
> patch hoping that somebody will pick it up or help me get it into shape.
>
It looks good. The signature change is not that bad if it forces all SAPIs to
be updated and ensures that PHP behaves the same way with all SAPIs.
It is also possible to add something like header_delete_handler() or
header_handler_ex() to sapi_module_struct if the signature change is to be
avoided.
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] parse_ini_string()
On Wednesday 05 November 2008 17:30:21 Olivier Grange-Labat wrote: > Hello, > > Here's a patch again PHP_5_3 to add a parse_ini_string() function. > > It just works as parse_ini_file(), except it accepts a string instead of > a filename, obviously. > > We've been using for months a simple PHP function to do that, and while > I had to modify it to accept constants (as parse_ini_file() does), I > thought it was time to use the core parsers instead of reinventing the > wheel. > > I have the same patch available for 5.2, if anyone is interested. > > Thank you for all the time and effort you put into PHP ! > > Olivier > Hi, Thanks, committed to 5.3 and HEAD :) Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PHP-CVS] cvs: php-src(PHP_5_3) / NEWS /ext/sockets sockets.c
On Thursday 23 October 2008 08:54:19 Hannes Magnusson wrote:
> On Wed, Oct 22, 2008 at 20:59, Arnaud Le Blanc <[EMAIL PROTECTED]> wrote:
> > lbarnaudWed Oct 22 18:59:34 2008 UTC
> >
> > Modified files: (Branch: PHP_5_3)
> >/php-srcNEWS
> >/php-src/ext/socketssockets.c
> > Log:
> > MFH: Fixed bug #46360 (TCP_NODELAY constant for socket_{get,set}_option)
> >
> >
> > http://cvs.php.net/viewvc.cgi/php-
src/NEWS?r1=1.2027.2.547.2.965.2.349&r2=1.2027.2.547.2.965.2.350&diff_format=u
> > Index: php-src/NEWS
> > diff -u php-src/NEWS:1.2027.2.547.2.965.2.349 php-
src/NEWS:1.2027.2.547.2.965.2.350
> > --- php-src/NEWS:1.2027.2.547.2.965.2.349 Tue Oct 21 23:22:00 2008
> > +++ php-src/NEWSWed Oct 22 18:59:33 2008
> > @@ -18,6 +18,8 @@
> >
> > - Fixed bug causing the algorithm parameter of mhash() to be modified.
(Scott)
> >
> > +- Fixed bug #46360 (TCP_NODELAY constant for socket_{get,set}_option).
> > + (bugs at trick dot vanstaveren dot us)
> > - Fixed bug #46238 (Segmentation fault on static call with empty string
method).
> > (Felipe)
> > - Fixed bug #46205 (Closure - Memory leaks when ReflectionException is
thrown).
> > http://cvs.php.net/viewvc.cgi/php-
src/ext/sockets/sockets.c?r1=1.171.2.9.2.14.2.7&r2=1.171.2.9.2.14.2.8&diff_format=u
> > Index: php-src/ext/sockets/sockets.c
> > diff -u php-src/ext/sockets/sockets.c:1.171.2.9.2.14.2.7 php-
src/ext/sockets/sockets.c:1.171.2.9.2.14.2.8
> > --- php-src/ext/sockets/sockets.c:1.171.2.9.2.14.2.7Tue Oct 21
23:39:15 2008
> > +++ php-src/ext/sockets/sockets.c Wed Oct 22 18:59:33 2008
> > @@ -19,7 +19,7 @@
> >
+--+
> > */
> >
> > -/* $Id: sockets.c,v 1.171.2.9.2.14.2.7 2008/10/21 23:39:15 lbarnaud Exp $
*/
> > +/* $Id: sockets.c,v 1.171.2.9.2.14.2.8 2008/10/22 18:59:33 lbarnaud Exp $
*/
> >
> > #ifdef HAVE_CONFIG_H
> > #include "config.h"
> > @@ -661,6 +661,9 @@
> >REGISTER_LONG_CONSTANT("SO_ERROR", SO_ERROR,
> >
CONST_CS | CONST_PERSISTENT);
> >REGISTER_LONG_CONSTANT("SOL_SOCKET",SOL_SOCKET,
CONST_CS | CONST_PERSISTENT);
> >REGISTER_LONG_CONSTANT("SOMAXCONN", SOMAXCONN,
> >
CONST_CS | CONST_PERSISTENT);
> > +#ifdef TCP_NODELAY
> > + REGISTER_LONG_CONSTANT("TCP_NODELAY", TCP_NODELAY,CONST_CS |
CONST_PERSISTENT);
> > +#endif
>
>
> I have cried so many times.
> Does anyone have any idea how we can help docwriters find commits that
> need to be documented?
>
> Personally I would like _all_ changes in functionality, new stuff and
> removed stuff to be attached to an bug report which then gets turned
> into a documentation bug report referencing the commit and preferably
> with a quick explanation from the developer on what changed including
> an example of usage.
>
> That would mean whenever you close a bug report you would get an extra
> step, asking if "the fix needs documenting" which would require answer
> and filling in the comment field before the report could be closed.
>
> That at least forces you to think, which the [DOC] tagging did not
> (and is probably the reason why it failed) and makes it 100% times
> easier for doc guys to do their work.
>
> -Hannes
>
Hi,
Sorry, I did not think to add a [DOC] tag in the commit message.
> That would mean whenever you close a bug report you would get an extra
> step, asking if "the fix needs documenting" which would require answer
> and filling in the comment field before the report could be closed.
+1 :)
Or at least something like "Convert to Documentation bug" close the the
"Submit" button on the bug form. Also, CC-ing the phpdoc mailing list while
creating a "Documentation" bug, or while closing a "Feature request" bug.
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Making ereg extension optional ?
Hi, PHP now builds and works without ereg, is it planed to make it optional ? Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Opinion needed (bug #45928)
On Wednesday 10 September 2008 11:25:40 Christian Schneider wrote:
> Am 10.09.2008 um 10:52 schrieb Tullio Andreatta ML:
> > fstat on a "open-ed fd" can't stat a symbolic link, since
> > open("symlink") returns a file descriptor of the target file, or -1
> > if it's a dangling link ...
>
>
> Yes, that's right. So Arnaud's patch at
http://arnaud.lb.s3.amazonaws.com/45928.patch
> looks fine to me.
> Someone with karma should commit this...
>
> One more thought: Maybe the same patch should also be applied to
> function zend_stream_stdio_fsizer() in the same file?
Committed, applied to zend_stream_stdio_fsizer() too.
>
> - Chris
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Opinion needed (bug #45928)
Hi,
On Tuesday 09 September 2008 17:35:54 Christian Schneider wrote:
> Arnaud Le Blanc wrote:
> > The following may (no MacOS X to test) fix the problem by returning 0 from
> > zend_stream_fsize() when the file descriptor is not a regular file:
> > http://arnaud.lb.s3.amazonaws.com/45928.patch
>
> Your patch:
> +#ifdef S_ISREG
> + if (!S_ISREG(buf.st_mode)) {
> + return 0;
> + }
> +#endif
> might prevent mmap-ing symbolically linked files, I'll do a quick test
> when I'm at home.
No, fstat() will resolve the link and stat the file the link points to. And I
believe script paths are fully resolved before being opened.
Anyway, given the fstat() Linux man page, its a bug to return st_size when the
file is not a regular file.
> Not sure if we should also check for symbolic links (and what they point
> to) or if that's overkill because the performance penalty is small
> enough anyway.
>
> Anyone from the core developers got an opinion here?
>
> - Chris
>
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Opinion needed (bug #45928)
Hi,
On Tuesday 09 September 2008 14:39:05 Alexey Zakhlestin wrote:
> http://bugs.php.net/bug.php?id=45928
>
> Christian Schneider wrote:
>
> > I had a quick look at this bug and found the problem to be in
> > Zend/zend_stream.c function zend_stream_fsize(): It uses fstat() to
> > determine the filesize which on MacOS X for pipes returns either 0 (my
> > interpretation: no data from the pipe ready yet) or a number up to 16384
> > (my interpretation: data from the pipe ready but the maximum buffer size
> > is 16k).
> >
> > I see several solutions but I'm not sure which is the desired one:
> > - return 0 (size unknown) if the file is a pipe (or socket, ...)
> > - return 0 if the file is not a regular file (or symlink, dir?)
> > - look into a way of determining EOF reached
> >
> > As a quick test I changed
> > return buf.st_size;
> > in function zend_stream_fsize() to
> > return 0;
> > and cat 30k.php | php worked after that.
> >
It seems to be a good solution. zend_stream seems to already rely on size == 0
for non-regular files and falls back to something like "while (!feof){ read }"
in that case.
The following may (no MacOS X to test) fix the problem by returning 0 from
zend_stream_fsize() when the file descriptor is not a regular file:
http://arnaud.lb.s3.amazonaws.com/45928.patch
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH][HEAD] Upload progress in sessions
Hi,
On Monday 08 September 2008 15:19:07 Richard Quadling wrote:
> 2008/9/8 Arnaud Le Blanc <[EMAIL PROTECTED]>:
> > Hi,
> >
> > On Monday 08 September 2008 13:06:50 Martin Jansen wrote:
> >> On Mon, Sep 8, 2008 at 12:18 PM, Arnaud Le Blanc <[EMAIL PROTECTED]>
> > wrote:
> >> > The patch allows to store upload progress informations in session
> > variables.
> >> > These informations can be retrieved by an other script while the upload
is
> >> > still in progress, allowing to provide feedback to the user.
> >> > Implementing this in the sessions extension makes this feature
available
> > to
> >> > every one.
> >> > Using the session extension for this purpose also allows to use the
> > different
> >> > storage backends available.
> >> > I have written a RFC/documentation here:
> >> > http://wiki.php.net/rfc/session_upload_progress
> >>
> >> Would it be possible to use something like
> >> $_SESSION['foo']['upload_progress_123'] instead of storing the
> >> information in the top-level session space? I can imagine there being
> >> frameworks and such that put their session stuff in some special
> >> container like $_SESSION['foo'] so that their session data does not
> >> interfere with the data of other components or the application that
> >> makes use of the framework.
> >
> > Yes, this can be done. The prefix setting ("upload_progress_" by default)
was
> > added for that purpose, but it may be possible to use $_SESSION[prefix]
[name].
> >
> >>
> >> Martin
> >>
> >
> > Regards,
> >
> > Arnaud
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: http://www.php.net/unsub.php
> >
> >
>
> Would maybe having $_SESSION['_Files'] == $_FILES for multiple uploads
> and have the current size uploaded as a new entry in $_FILES be
> easier?
>
> That way no additional configuration option.
This will not work if the user sends a request while a previous one is not
terminated. Also, this may break some applications that use "_Files" as a key
in $_SESSION if this is not configurable. And the entries outside of "files"
(content_length, etc) needs to be added somewhere ;)
The "files" array in the current patch is just like $_FILES.
>
>
>
> --
> -
> Richard Quadling
> Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
> "Standing on the shoulders of some very clever giants!"
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
>
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH][HEAD] Upload progress in sessions
Hi,
On Monday 08 September 2008 13:06:50 Martin Jansen wrote:
> On Mon, Sep 8, 2008 at 12:18 PM, Arnaud Le Blanc <[EMAIL PROTECTED]>
wrote:
> > The patch allows to store upload progress informations in session
variables.
> > These informations can be retrieved by an other script while the upload is
> > still in progress, allowing to provide feedback to the user.
> > Implementing this in the sessions extension makes this feature available
to
> > every one.
> > Using the session extension for this purpose also allows to use the
different
> > storage backends available.
> > I have written a RFC/documentation here:
> > http://wiki.php.net/rfc/session_upload_progress
>
> Would it be possible to use something like
> $_SESSION['foo']['upload_progress_123'] instead of storing the
> information in the top-level session space? I can imagine there being
> frameworks and such that put their session stuff in some special
> container like $_SESSION['foo'] so that their session data does not
> interfere with the data of other components or the application that
> makes use of the framework.
Yes, this can be done. The prefix setting ("upload_progress_" by default) was
added for that purpose, but it may be possible to use $_SESSION[prefix][name].
>
> Martin
>
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH][HEAD] Upload progress in sessions
Hi, On Monday 08 September 2008 13:03:11 Pierre Joye wrote: > On Mon, Sep 8, 2008 at 12:18 PM, Arnaud Le Blanc <[EMAIL PROTECTED]> wrote: > > Hi, > > > > I have written a patch to implement upload progress feedback in session data. > > > > The file upload feature in PHP allows extensions to be called back on some > > events. A few extensions use this to implement some sort of upload progress > > feedback, but none of them are released with PHP, which makes this feature > > unusable in many environments. > > > > The patch allows to store upload progress informations in session variables. > > These informations can be retrieved by an other script while the upload is > > still in progress, allowing to provide feedback to the user. > > Implementing this in the sessions extension makes this feature available to > > every one. > > Using the session extension for this purpose also allows to use the different > > storage backends available. > > I have written a RFC/documentation here: > > http://wiki.php.net/rfc/session_upload_progress > > > > Any objection on committing this to HEAD ? > > It is indeed nice to have this feature without relying on an > extension, however do you know that APC will be bundled in php6? As > APC provides this feature. Yes, but I don't think it will be enabled on shared hosting, for instance. Also, while session data can be shared across multiple servers, APC's storage can't. > > Cheers, > -- > Pierre > http://blog.thepimp.net | http://www.libgd.org > Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH][HEAD] Upload progress in sessions
Hi, I have written a patch to implement upload progress feedback in session data. The file upload feature in PHP allows extensions to be called back on some events. A few extensions use this to implement some sort of upload progress feedback, but none of them are released with PHP, which makes this feature unusable in many environments. The patch allows to store upload progress informations in session variables. These informations can be retrieved by an other script while the upload is still in progress, allowing to provide feedback to the user. Implementing this in the sessions extension makes this feature available to every one. Using the session extension for this purpose also allows to use the different storage backends available. I have written a RFC/documentation here: http://wiki.php.net/rfc/session_upload_progress Any objection on committing this to HEAD ? Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] BUG #45392
Hi, On Tuesday 02 September 2008 12:15:02 Dmitry Stogov wrote: > > Jani Taskinen wrote: > >>> Dmitry Stogov wrote: > Hi, > > Attached is a proposed fix for http://bugs.php.net/bug.php?id=45392 for > PHP_5_3. I don't know output buffering part very well, and I'm not > completely sure about this fix, so please review it. > > The patch changes behavior of output buffering a bit. In case of fatal > error all output buffers opened by ob_start() with zero (or omitted) > chunk_size argument are discarded. The fix brakes two tests: > > Test session_module_name() function : variation > [ext/session/tests/session_module_name_variation3.phpt] > Test session_set_save_handler() function : error functionality > [ext/session/tests/session_set_save_handler_error3.phpt] > > We need to make a decision about #45392. > > 1) Fix it with proposed (or similar) patch > > 2) Make it bogus because any fix requires output buffering behavior > change > > > > Sorry for top-posting. Anyway, reading the patch made me think that > > maybe in the user/recoverable cases the output buffer should not be > > discarded like this..? > > > > And IMO, it's a bug, bugs should be fixed even if it means changing the > > behaviour in an _edge_ case. > > I'm not so sure as http://www.php.net/manual/en/function.ob-start.php > doesn't make difference between successful and unsuccessful request and > assumes "output buffer is flushed to the browser at the end of the request". I think that it is critical to output this kind of buffer in case of fatal error, as it may unexpectedly leak data which was not meant to be output. It seems Ok to me to discard this kind of buffers in this case. > > > How do those session tests fail..? > > As they have ob_start() at top, the output is discarded and only error > message is printed. > > > They expect output with fatal errors? (actually those tests failed > > without your patch too, IIRC :) > > They works for me without the patch. > > Thanks. Dmitry. > > > --Jani > > > Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Native TLS for globals in ZTS
Hi, I updated the RFC. I added benchmark tests on x86_64: Position independent code is the default there and is as fast as non-PIC on IA-32. The initial patch using dynamic TLS gives the same results as on IA-32 with static TLS. I also tested the patch (with a few changes) on Windows Server 2008 with IIS (older Windows cannot use TLS in LoadLibrary()-loaded DLLs). This works as expected, but there are still some restrictions. Regards, Arnaud On Sunday 24 August 2008 18:37:09 Arnaud Le Blanc wrote: > Hi, > > I have written a RFC about my patch at http://wiki.php.net/rfc/tls > > The goal of this patch is to reduce the performance overhead of ZTS builds. > > Since the initial patch I made more research on various implementations of > TLS: Linux, FreeBSD, Solaris, Windows. Based on what I found I wrote a new > patch to avoid the problems of the initial one and the results on bench.php > are as follows: > > > non-ZTS > 3.7s > > > ZTS unpatched >5.2s > > > ZTS-patched, native TLS disabled > 5.0s > > > ZTS-patched, native TLS enabled > 4.2s > > > Regards, > > Arnaud > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Something to ponder on ZTS/TSRM
Hi, On Friday 22 August 2008 21:15:56 William A. Rowe, Jr. wrote: > As far as the future direction of embedded PHP (and let's agree here > we aren't talking about every application, for mass vhosters some > fcgi or suid flavor of PHP is going to remain a better choice, and > for others, so we can avoid that debate)... > > The next direction for server worker pools will likely be a threadless > connection; while waiting for your POST body the php engine environment > will most efficiently be parked and resumed on another thread. (Some > code has been started in this direction at httpd although there are > no corresponding 'engine' hacks yet for embedded language systems.) > > So although the new patches look great for a threading application, > keep in mind that a more traditional zts mode may be worthwhile to > retain. The patch, if it is included, does not remove the current ZTS mode ;) However the current mode does not guaranties that PHP will work if a request is started in a thread and continued in an other. If support for that is added in the future (TSRM already has a set of functions to allow that), this can work with the patch too. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] Native TLS for globals in ZTS
Hi, I have written a RFC about my patch at http://wiki.php.net/rfc/tls The goal of this patch is to reduce the performance overhead of ZTS builds. Since the initial patch I made more research on various implementations of TLS: Linux, FreeBSD, Solaris, Windows. Based on what I found I wrote a new patch to avoid the problems of the initial one and the results on bench.php are as follows: non-ZTS 3.7s ZTS unpatched 5.2s ZTS-patched, native TLS disabled 5.0s ZTS-patched, native TLS enabled 4.2s Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, On Wednesday 20 August 2008 21:51:05 Kalle Sommer Nielsen wrote: > 2008/8/20 Arnaud Le Blanc <[EMAIL PROTECTED]>: > > Hi, > > > > On Tuesday 19 August 2008 18:22:44 Andi Gutmans wrote: > >> OK checked with Zeev. It seems there are some significant limitations at > >> least on Windows including that it doesn't work with LoadLibrary() > >> (which is our bread and butter). > > > > Ok, so there is the same limitations than on Linux with dlopen() :( > > > >> There may also be some size limitations on thread local storage. > >> In any case, this is Windows-only feedback and we may find additional > >> limitations/compatibility issues on other platforms. > >> > >> As there are clear benefits to an increased use of TLS (we already use > >> it for index caching today) I definitely suggest to revisit this issue > >> and try and figure out for the variety of platforms whether there's some > >> middle ground that we can make work. It sounds like a non-trivial > >> project though. > >> Arnaud, are you setup to also play around with Windows and possibly some > >> other OSes to look into this further? > > > > Yes, I will try at least with Windows (XP, so no IIS) and FreeBSD. > > > > Windows XP Professional have IIS, all you need is your CD and then > install it from the Add/Remove programs. It's IIS 5.1, which should be > good enough. Thanks, I didn't known that. I did not have a Professional version of Windows but I will try to get one. > > >> > >> Andi > >> > >> > >> > -Original Message- > >> > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > >> > Sent: Monday, August 18, 2008 10:00 PM > >> > To: Andi Gutmans > >> > Cc: PHP Development > >> > Subject: Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > >> > > >> > Hi, > >> > > >> > Yes, I have looked for the issue with --with-tsrm-full-__thread-tls > >> and there > >> > are effectively some issues. > >> > > >> > When building PIC code, the used TLS model is a static model which > >> does not > >> > allow modules to be loaded at run-time. glibc's dlopen() sometimes > >> allow such > >> > code to be loaded at runtime when it finds some free memory, that's > >> why -- > >> > with- > >> > tsrm-__thread-tls works, but it is not expected to always work. > >> > > >> > So when building PIC code that's expected to work only when the > >> > server/application links the PHP module, or when using LD_PRELOAD, > >> etc. > >> > > >> > Building non-PIC code allows to use a dynamic TLS model, which allows > >> to load > >> > modules a run-time, but it is less efficient (4.8s in bench.php, still > >> faster > >> > than unpatched version, even non-PIC, but less efficient). > >> > > >> > Regards, > >> > > >> > Arnaud > >> > > >> > On Tuesday 19 August 2008 06:18:51 Andi Gutmans wrote: > >> > > Hi Arnaud, > >> > > > >> > > I remember that at the time we looked at thread local storage and > >> there > >> > > were some real issues with it. I can't remember what as it was about > >> 7+ > >> > > years ago. > >> > > I will ask Zeev if he remembers and if not search my archives (don't > >> > > have years prior to 2007 indexed :'( ). > >> > > > >> > > Andi > >> > > > >> > > > >> > > > >> > > > >> > > > -Original Message- > >> > > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > >> > > > Sent: Saturday, August 16, 2008 7:19 PM > >> > > > To: PHP Development > >> > > > Subject: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > >> > > > > >> > > > Hi, > >> > > > > >> > > > Currently the way globals work forces to pass a > >> thread-local-storage > >> > > pointer > >> > > > across function calls, which involves some overhead. Also, not all > >> > > functions > >> > > > get the pointer as argument and need to use TSRMLS_FETCH(), which > >> is > >> > > slow. For > >> > > > instance emalloc()
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, On Thursday 21 August 2008 09:37:12 Dmitry Stogov wrote: > > Arnaud Le Blanc wrote: > > Hi, > > > > On Tuesday 19 August 2008 09:22:46 Dmitry Stogov wrote: > >> Hi Arnaud, > >> > >> Arnaud Le Blanc wrote: > >>> Hi, > >>> > >>> On Monday 18 August 2008 19:46:46 Dmitry Stogov wrote: > >>>> Hi Arnaud, > >>>> > >>>> The patch looks very interesting. > >>>> I think it may be committed to the HEAD in the nearest future. > >>>> I don't have time to look into all details in the moment. > >>>> > >>>> Could you explain why --with-tsrm-full-__thread-tls doesn't work with > >>>> dlopen() however --with-tsrm-__thread-tls does? > >>> That's due to the way TLS works internally. Actually I need further > > reading on > >>> that. > >> I don't see a big difference between --with-tsrm-full-__thread-tls and > >> --with-tsrm-__thread-tls from TLS point of view (may be I miss it), so I > >> don't understand why one works and the other doesn't. > > > > Badly both was not expected to work with dlopen() :( > > http://marc.info/?l=php-internals&m=121912220705964&w=2 > > > > It works when using an other TLS model (which requires PIC). > > This model is less efficient but it still improves performance. PIC-patched is > > faster than non-PIC-unpatched, and the improvement is even greater against > > PIC-unpatched. > > > > This is the thing I was afraid. > > >> The patch looks more difficult for me than it should. I would prefer to > >> have only --with-tsrm-full-__thread-tls, if it works, as the patch would > >> be simple and PHP faster. > >> > >> Another simple solution, which you probably already tested, is to use > >> only global __thread-ed tsrm_ls (and don't pass/fetch it), however, > >> access thread-globals in the same way: > >> ((*type)tsrm_ls[global_module_id])->global_fileld > > > > Yes, I tested and it given 4.7s on bench.php (vs 3.8s with the current patch). > > > > Does this model have the same issues with dlopen()? > (We have only one __thread-ed variable). Yes and no. By reading the loader/linker code in the glibc I seen that it reserves a number of memory especially for the dlopen() case. This number of memory it too small for --with-tsrm-full-__thread-tls, but several times larger than needed for --with-tsrm-__thread-tls. So the memory found by dlopen() is not random and I think we can expect that to work on Linux. However things are different on Windows and FreeBSD. I will post a RFC about that to clarify things. > > Thanks. Dmitry. > > >> Anyway you did a great job. I would like to see this idea implemented in > >> HEAD. It is little bit late for 5.3 :( > >> > >> Thanks. Dmitry. > >> > >>>> Did you test the patch with DSO extensions? > >>> I will, but I guess that will be behaves like another shared library > >>> dlopen()ed by Apache. > >>> > >>>> It would be interesting to try the same idea on Windows with VC. > >>> I will try too. > >>> > >>>> Thanks. Dmitry. > >>>> > >>>> Arnaud Le Blanc wrote: > >>>>> Hi, > >>>>> > >>>>> Currently the way globals work forces to pass a thread-local-storage > >>> pointer > >>>>> across function calls, which involves some overhead. Also, not all > >>> functions > >>>>> get the pointer as argument and need to use TSRMLS_FETCH(), which is > > slow. > >>> For > >>>>> instance emalloc() involves a TSRMLS_FETCH(). An other overhead is > >>> accessing > >>>>> globals, using multiple pointers in different locations. > >>>>> > >>>>> The following patch caches each global address in a native TLS variable > > so > >>>>> that accessing a global is as simple as global_name->member. This > > removes > >>> the > >>>>> requirement of passing the tls pointer across function calls, so that > > the > >>> two > >>>>> major overheads of ZTS builds are avoided. > >>>>> > >>>>> Globals can optionally be declared statically, which speeds up things a > >>> bit. > >>>>> Results in bench.php: > >>>>&g
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, On Tuesday 19 August 2008 09:52:24 Uwe Schindler wrote: > I do not know exactly how this will behave with other web servers. I do the > module for Sun Java System Webservers (NSAPI) which normally uses pthreads > (on Solaris, Linux) but not on AIX (this is why PHP as NSAPI module does not > work on AIX). > > The correct way for this webserver would be to use the macros/functions from > the NSAPI library to handle threads, which is implemented in TSRM [see > #ifdef NSAPI], but never used, because when compiling PHP the "NSAPI" > defines are only known to the NSAPI module, but not to the whole source tree > - so TSRM uses pthreads (which is clear then). The parallel CLI version of > PHP compiled with NSAPI threads would not work, too (if not using pthreads). > > But for newer SJSWS servers, this is not a problem, if PHP's NSAPI always > uses pthreads - only AIX is broken (but this since years). > > So: may there be issues with this server, too? I would like to test it, too, > but I need to eventually add code to sapi/nsapi, you may help me :) I think there will be nothing to change in SAPIs to work with that as long as the platform supports __thread. I think the patch should work as-is on Linux (with PHP compiled with --with-pic). It seems Solaris has an implementation very close to the one used on Linux, so it should work on Solaris too. Regards, Arnaud > > - > Uwe Schindler > [EMAIL PROTECTED] - http://www.php.net > NSAPI SAPI developer > Bremen, Germany > > > -Original Message- > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > > Sent: Tuesday, August 19, 2008 7:00 AM > > To: Andi Gutmans > > Cc: PHP Development > > Subject: Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > > > > Hi, > > > > Yes, I have looked for the issue with --with-tsrm-full-__thread-tls and > > there > > are effectively some issues. > > > > When building PIC code, the used TLS model is a static model which does > > not > > allow modules to be loaded at run-time. glibc's dlopen() sometimes allow > > such > > code to be loaded at runtime when it finds some free memory, that's why -- > > with- > > tsrm-__thread-tls works, but it is not expected to always work. > > > > So when building PIC code that's expected to work only when the > > server/application links the PHP module, or when using LD_PRELOAD, etc. > > > > Building non-PIC code allows to use a dynamic TLS model, which allows to > > load > > modules a run-time, but it is less efficient (4.8s in bench.php, still > > faster > > than unpatched version, even non-PIC, but less efficient). > > > > Regards, > > > > Arnaud > > > > On Tuesday 19 August 2008 06:18:51 Andi Gutmans wrote: > > > Hi Arnaud, > > > > > > I remember that at the time we looked at thread local storage and there > > > were some real issues with it. I can't remember what as it was about 7+ > > > years ago. > > > I will ask Zeev if he remembers and if not search my archives (don't > > > have years prior to 2007 indexed :'( ). > > > > > > Andi > > > > > > > > > > > > > > > > -Original Message- > > > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > > > > Sent: Saturday, August 16, 2008 7:19 PM > > > > To: PHP Development > > > > Subject: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > > > > > > > > Hi, > > > > > > > > Currently the way globals work forces to pass a thread-local-storage > > > pointer > > > > across function calls, which involves some overhead. Also, not all > > > functions > > > > get the pointer as argument and need to use TSRMLS_FETCH(), which is > > > slow. For > > > > instance emalloc() involves a TSRMLS_FETCH(). An other overhead is > > > accessing > > > > globals, using multiple pointers in different locations. > > > > > > > > The following patch caches each global address in a native TLS > > > variable so > > > > that accessing a global is as simple as global_name->member. This > > > removes the > > > > requirement of passing the tls pointer across function calls, so that > > > the two > > > > major overheads of ZTS builds are avoided. > > > > > > > > Globals can optionally be declared statically, which speeds up things > > > a bit. > > > > > > > > Results in bench.php: > >
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, On Tuesday 19 August 2008 18:22:44 Andi Gutmans wrote: > OK checked with Zeev. It seems there are some significant limitations at > least on Windows including that it doesn't work with LoadLibrary() > (which is our bread and butter). Ok, so there is the same limitations than on Linux with dlopen() :( > There may also be some size limitations on thread local storage. > In any case, this is Windows-only feedback and we may find additional > limitations/compatibility issues on other platforms. > > As there are clear benefits to an increased use of TLS (we already use > it for index caching today) I definitely suggest to revisit this issue > and try and figure out for the variety of platforms whether there's some > middle ground that we can make work. It sounds like a non-trivial > project though. > Arnaud, are you setup to also play around with Windows and possibly some > other OSes to look into this further? Yes, I will try at least with Windows (XP, so no IIS) and FreeBSD. > > Andi > > > > -Original Message- > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > > Sent: Monday, August 18, 2008 10:00 PM > > To: Andi Gutmans > > Cc: PHP Development > > Subject: Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > > > > Hi, > > > > Yes, I have looked for the issue with --with-tsrm-full-__thread-tls > and there > > are effectively some issues. > > > > When building PIC code, the used TLS model is a static model which > does not > > allow modules to be loaded at run-time. glibc's dlopen() sometimes > allow such > > code to be loaded at runtime when it finds some free memory, that's > why -- > > with- > > tsrm-__thread-tls works, but it is not expected to always work. > > > > So when building PIC code that's expected to work only when the > > server/application links the PHP module, or when using LD_PRELOAD, > etc. > > > > Building non-PIC code allows to use a dynamic TLS model, which allows > to load > > modules a run-time, but it is less efficient (4.8s in bench.php, still > faster > > than unpatched version, even non-PIC, but less efficient). > > > > Regards, > > > > Arnaud > > > > On Tuesday 19 August 2008 06:18:51 Andi Gutmans wrote: > > > Hi Arnaud, > > > > > > I remember that at the time we looked at thread local storage and > there > > > were some real issues with it. I can't remember what as it was about > 7+ > > > years ago. > > > I will ask Zeev if he remembers and if not search my archives (don't > > > have years prior to 2007 indexed :'( ). > > > > > > Andi > > > > > > > > > > > > > > > > -Original Message- > > > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > > > > Sent: Saturday, August 16, 2008 7:19 PM > > > > To: PHP Development > > > > Subject: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > > > > > > > > Hi, > > > > > > > > Currently the way globals work forces to pass a > thread-local-storage > > > pointer > > > > across function calls, which involves some overhead. Also, not all > > > functions > > > > get the pointer as argument and need to use TSRMLS_FETCH(), which > is > > > slow. For > > > > instance emalloc() involves a TSRMLS_FETCH(). An other overhead is > > > accessing > > > > globals, using multiple pointers in different locations. > > > > > > > > The following patch caches each global address in a native TLS > > > variable so > > > > that accessing a global is as simple as global_name->member. This > > > removes the > > > > requirement of passing the tls pointer across function calls, so > that > > > the two > > > > major overheads of ZTS builds are avoided. > > > > > > > > Globals can optionally be declared statically, which speeds up > things > > > a bit. > > > > > > > > Results in bench.php: > > > > non-ZTS:3.7s > > > > ZTS unpatched: 5.2s > > > > ZTS patched:4.0s > > > > ZTS patched and static globals: 3.8s > > > > > > > > The patch introduces two new macros: TSRMG_D() (declare) and > > > TSRMG_DH() > > > > (declare, for headers) to declare globals, instead of the current > > > &
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, On Tuesday 19 August 2008 09:22:46 Dmitry Stogov wrote: > Hi Arnaud, > > Arnaud Le Blanc wrote: > > Hi, > > > > On Monday 18 August 2008 19:46:46 Dmitry Stogov wrote: > >> Hi Arnaud, > >> > >> The patch looks very interesting. > >> I think it may be committed to the HEAD in the nearest future. > >> I don't have time to look into all details in the moment. > >> > >> Could you explain why --with-tsrm-full-__thread-tls doesn't work with > >> dlopen() however --with-tsrm-__thread-tls does? > > > > That's due to the way TLS works internally. Actually I need further reading on > > that. > > I don't see a big difference between --with-tsrm-full-__thread-tls and > --with-tsrm-__thread-tls from TLS point of view (may be I miss it), so I > don't understand why one works and the other doesn't. Badly both was not expected to work with dlopen() :( http://marc.info/?l=php-internals&m=121912220705964&w=2 It works when using an other TLS model (which requires PIC). This model is less efficient but it still improves performance. PIC-patched is faster than non-PIC-unpatched, and the improvement is even greater against PIC-unpatched. > > The patch looks more difficult for me than it should. I would prefer to > have only --with-tsrm-full-__thread-tls, if it works, as the patch would > be simple and PHP faster. > > Another simple solution, which you probably already tested, is to use > only global __thread-ed tsrm_ls (and don't pass/fetch it), however, > access thread-globals in the same way: > ((*type)tsrm_ls[global_module_id])->global_fileld Yes, I tested and it given 4.7s on bench.php (vs 3.8s with the current patch). > > Anyway you did a great job. I would like to see this idea implemented in > HEAD. It is little bit late for 5.3 :( > > Thanks. Dmitry. > > >> Did you test the patch with DSO extensions? > > > > I will, but I guess that will be behaves like another shared library > > dlopen()ed by Apache. > > > >> It would be interesting to try the same idea on Windows with VC. > > > > I will try too. > > > >> Thanks. Dmitry. > >> > >> Arnaud Le Blanc wrote: > >>> Hi, > >>> > >>> Currently the way globals work forces to pass a thread-local-storage > > pointer > >>> across function calls, which involves some overhead. Also, not all > > functions > >>> get the pointer as argument and need to use TSRMLS_FETCH(), which is slow. > > For > >>> instance emalloc() involves a TSRMLS_FETCH(). An other overhead is > > accessing > >>> globals, using multiple pointers in different locations. > >>> > >>> The following patch caches each global address in a native TLS variable so > >>> that accessing a global is as simple as global_name->member. This removes > > the > >>> requirement of passing the tls pointer across function calls, so that the > > two > >>> major overheads of ZTS builds are avoided. > >>> > >>> Globals can optionally be declared statically, which speeds up things a > > bit. > >>> Results in bench.php: > >>> non-ZTS: 3.7s > >>> ZTS unpatched:5.2s > >>> ZTS patched: 4.0s > >>> ZTS patched and static globals: 3.8s > >>> > >>> The patch introduces two new macros: TSRMG_D() (declare) and TSRMG_DH() > >>> (declare, for headers) to declare globals, instead of the current > > "ts_rsrc_id > >>> foo_global_id". These macros declare the global id, plus the __thread > > pointer > >>> to the global storage. > >>> > >>> ts_allocate_id now takes one more callback function as argument to bind > > the > >>> global pointer to its storage. This callback is declared in TSRMG_D[H] (). > >>> > >>> As all TSRMLS_* macros now does nothing, it is needed to call > > ts_resource(0) > >>> explicitly at least one time in each thread to initialize its storage. A > > new > >>> TSRMLS_INIT() macro as been added for this purpose. > >>> > >>> All this is disabled by default. --with-tsrm-__thread-tls enables the > > features > >>> of the patch, and --with-tsrm-full-__thread-tls enables static declaration > > of > >>> globals. > >>> > >&g
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
Hi, Yes, I have looked for the issue with --with-tsrm-full-__thread-tls and there are effectively some issues. When building PIC code, the used TLS model is a static model which does not allow modules to be loaded at run-time. glibc's dlopen() sometimes allow such code to be loaded at runtime when it finds some free memory, that's why --with- tsrm-__thread-tls works, but it is not expected to always work. So when building PIC code that's expected to work only when the server/application links the PHP module, or when using LD_PRELOAD, etc. Building non-PIC code allows to use a dynamic TLS model, which allows to load modules a run-time, but it is less efficient (4.8s in bench.php, still faster than unpatched version, even non-PIC, but less efficient). Regards, Arnaud On Tuesday 19 August 2008 06:18:51 Andi Gutmans wrote: > Hi Arnaud, > > I remember that at the time we looked at thread local storage and there > were some real issues with it. I can't remember what as it was about 7+ > years ago. > I will ask Zeev if he remembers and if not search my archives (don't > have years prior to 2007 indexed :'( ). > > Andi > > > > > > -Original Message- > > From: Arnaud Le Blanc [mailto:[EMAIL PROTECTED] > > Sent: Saturday, August 16, 2008 7:19 PM > > To: PHP Development > > Subject: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS > > > > Hi, > > > > Currently the way globals work forces to pass a thread-local-storage > pointer > > across function calls, which involves some overhead. Also, not all > functions > > get the pointer as argument and need to use TSRMLS_FETCH(), which is > slow. For > > instance emalloc() involves a TSRMLS_FETCH(). An other overhead is > accessing > > globals, using multiple pointers in different locations. > > > > The following patch caches each global address in a native TLS > variable so > > that accessing a global is as simple as global_name->member. This > removes the > > requirement of passing the tls pointer across function calls, so that > the two > > major overheads of ZTS builds are avoided. > > > > Globals can optionally be declared statically, which speeds up things > a bit. > > > > Results in bench.php: > > non-ZTS:3.7s > > ZTS unpatched: 5.2s > > ZTS patched:4.0s > > ZTS patched and static globals: 3.8s > > > > The patch introduces two new macros: TSRMG_D() (declare) and > TSRMG_DH() > > (declare, for headers) to declare globals, instead of the current > "ts_rsrc_id > > foo_global_id". These macros declare the global id, plus the __thread > pointer > > to the global storage. > > > > ts_allocate_id now takes one more callback function as argument to > bind the > > global pointer to its storage. This callback is declared in > TSRMG_D[H](). > > > > As all TSRMLS_* macros now does nothing, it is needed to call > ts_resource(0) > > explicitly at least one time in each thread to initialize its storage. > A new > > TSRMLS_INIT() macro as been added for this purpose. > > > > All this is disabled by default. --with-tsrm-__thread-tls enables the > features > > of the patch, and --with-tsrm-full-__thread-tls enables static > declaration of > > globals. > > > > It as been tested on Linux compiled with --disable-all in CLI and a > bit in > > Apache2 with the worker MPM. Known issues: > > - Declaring globals statically (--with-tsrm-full-__thread-tls) causes > troubles > > to dlopen(), actually Apache wont load the module at runtime (it works > with > > just --with-tsrm-__thread-tls). > > - The patch assumes that all resources are ts_allocate_id()'ed before > any > > other thread calls ts_allocate_id or ts_resource_ex(), which is > possibly not > > the case. > > > > The patch needs some tweaks and does not pretend to be included in any > branch, > > but I would like to have some comments on it. > > > > The patch: http://arnaud.lb.s3.amazonaws.com/__thread-tls.patch > > > > Regards, > > > > Arnaud > > > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
