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 a...@zend.com 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--
+?php
+$a = val_of_a;
+$b = val_of_b;
+
+echo -- 1 --\n;
+var_dump(true ? foo : bar);
+true ? foo : bar;
+$z = true ? foo : bar;
+
+echo -- 2 --\n;
+var_dump(true ? foo : c.d);
+true ? foo : c.d;
+$z = true ? foo : c.d;
+
+echo -- 3 --\n;
+var_dump(true ? foo : $b);
+true ? foo : $b;
+$z = true ? foo : $b;
+
+echo -- 4 --\n;
+var_dump(true ? foo : ${'b'});
+true ? foo : ${'b'};
+$z = true ? foo : ${'b'};
+
+echo -- 5 --\n;
+var_dump(true ? a.b : bar);
+true ? a.b : bar;
+$z = true ? a.b : bar;
+
+echo -- 6 --\n;
+var_dump(true ? a.b : c.d);
+true ? a.b : c.d;
+$z = true ? a.b : c.d;
+
+echo -- 7 --\n;
+var_dump(true ? a.b : $b);
+true ? a.b : $b;
+$z = true ? a.b : $b;
+
+echo -- 8 --\n;
+var_dump(true ? a.b : ${'b'});
+true ? a.b : ${'b'};
+$z = true ? a.b : ${'b'};
+
+echo -- 9 --\n;
+var_dump(true ? $a : bar);
+true ? $a : bar;
+$z = true ? $a : bar;
+
+echo -- 10 --\n;
+var_dump(true ? $a : c.d);
+true ? $a : c.d;
+$z = true ? $a : c.d;
+
+echo -- 11 --\n;
+var_dump(true ? $a : $b);
+true ? $a : $b;
+$z = true ? $a : $b;
+
+echo -- 12 --\n;
+var_dump(true ? $a : ${'b'});
+true ? $a : ${'b'};
+$z = true ? $a : ${'b'};
+
+echo -- 13 --\n;
+var_dump(true ? ${'a'} : bar);
+true ? ${'a'} : bar;
+$z = true ? ${'a'} : bar;
+
+echo -- 14 --\n;
+var_dump(true ? ${'a'} : c.d);
+true ? ${'a'} : c.d;
+$z = true ? ${'a'} : c.d;
+
+echo -- 15 --\n;
+var_dump(true ? ${'a'} : $b);
+true ? ${'a'} : $b;
+$z = true ? ${'a'} : $b;
+
+echo -- 16 --\n;
+var_dump(true ? ${'a'} : ${'b'});
+true ? ${'a'} : ${'b'};
+$z = true ? ${'a'} : ${'b'};
+
+echo -- 17 --\n;
+var_dump(false ? foo : bar);
+false ? foo : bar;
+$z = false ? foo : bar;
+
+echo -- 18 --\n;
+var_dump(false ? foo : c.d);
+false ? foo : c.d;
+$z = false ? foo : c.d;
+
+echo -- 19 --\n;
+var_dump(false ? foo : $b);
+false ? foo : $b;
+$z = false ? foo : $b;
+
+echo -- 20 --\n;
+var_dump(false ? foo : ${'b'});
+false ? foo : ${'b'};
+$z = false ? foo : ${'b'};
+
+echo -- 21 --\n;
+var_dump(false ? a.b : bar);
+false ? a.b : bar;
+$z = false ? a.b : bar;
+
+echo -- 22 --\n;
+var_dump(false
[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?
?php
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 smalys...@sugarcrm.com 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 konfere...@kukulich.cz: 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
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
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
[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 lbarn...@php.net 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 lbarn...@php.net 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 http_fopen_wrapper.c.patch.txt-- 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] 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 lbarn...@php.net 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 lbarn...@php.net 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 http_fopen_wrapper.c.patch.txt-- 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
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 lbarn...@php.net 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 http_fopen_wrapper.c.patch.txt-- 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: 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 lbarn...@php.net 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
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] 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 lbarn...@php.net 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] PHP 5.3.0RC2
On Mon, May 4, 2009 at 5:29 PM, Lukas Kahwe Smith m...@pooteeweet.org 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.
Hi, On Mon, May 4, 2009 at 9:36 AM, shire sh...@php.net 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] [PATCH] Scanner diet with fixes, etc.
On Mon, May 4, 2009 at 5:51 PM, shire sh...@php.net wrote:
Arnaud Le Blanc wrote:
Hi,
On Mon, May 4, 2009 at 9:36 AM, shiresh...@php.net 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] 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-internalsm=121442930703916w=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-internalsm=121442930703916w=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] [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.349r2=1.2027.2.547.2.965.2.350diff_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.7r2=1.171.2.9.2.14.2.8diff_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 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] 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
[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] [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
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 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=ZEND002498r=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] 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
[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] 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
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-internalsm=121912220705964w=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: 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
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() 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
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: 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
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-internalsm=121912220705964w=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. 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 Regards, Arnaud 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 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 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
Re: [PHP-DEV] [PATCH] ZTS as fast as non-ZTS
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. 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. 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 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 Monday 18 August 2008 22:26:20 Stanislav Malyshev wrote: Hi! 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. I think it would be great to use __thread there. But I think if we have working __thread, why not have real globals use it, without all that TSRMG stuff? Having 3 different variants of TSRM support seems excessive. I'm agree with you, but actually TSRM does more that just allocating and storing globals. For instance it keeps track of constructors and destructors so that it can call them automatically when a new thread starts or stops. It also allows to retrieve the globals of an other thread, etc. Now, the question is can we reliably detect if we have working __thread - or, in other words, are there compilers which would accept __thread but do not implement it correctly, and can those be identified automatically? For that I checked how the glibc chooses to use __thread or not. Actually it just tries to compile a source like I do in the patch. But as TLS handling needs the help of the libc itself, and the glibc knows it can handle TLS, I guess we will need to do write a more complete test to check if it works and if it works as we expect it to work. If we use static declaration with __thread, then as far as I can see there is no need for separate IDs and all complications following from that. - 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). What is the problem there, could you elaborate? -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] 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, 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
[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-DEV] [PATCH] Bug #45384, ini parse error with no trailing newline
Hi, Here is a patch for #45384 (the ini parser returns a parse error when it hits EOF while parsing a non enclosed string): http://arnaud.lb.s3.amazonaws.com/parse_ini_file_45384.patch Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Zend Signal Handling
On Thursday 14 August 2008 02:59:48 Arnaud Le Blanc wrote: Hi, I made some changes to the patch: http://arnaud.lb.s3.amazonaws.com/php-5.3.0-alarms-0808141122.patch - Apache effectively seems to resets the signals after MINIT, so original handlers are now saved in RINIT in the first request of the process. - Removed some unneeded blocks/unblocks. - Changed all added TSRMLS_FETCH() so that they affect only ZTS+ZEND_SIGNAL builds. - Removed some added TSRMLS_FETCH() (all in zend_alloc and a few in zend_hash). Regards, Arnaud I made an other patch that caches tsrm_ls in a thread-local variable when calling TSRMLS_FETCH(), on platforms that support the __thread specifier: http://arnaud.lb.s3.amazonaws.com/tsrm_ls_cache.patch This gives a 10 to 15% speedup to ZTS builds on bench.php. It also makes ZEND_SIGNAL+ZTS builds run as fast as ZTS without ZEND_SIGNAL. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Zend Signal Handling
Hi, On Wednesday 13 August 2008 10:09:43 Dmitry Stogov wrote: Hi Lucas, I took a look into patch and I still don't like it. I may miss some things and make mistakes so correct me if I'm wrong. 1) It makes some slowdown for all SAPIs except Apache1, because it adds additional block/unblock code (mainly in zend_alloc.c) The added block/unblocks are really needed, for instance in _zend_mm_alloc_int when a small free block is found, it has to maintain the cache, etc and an interruption here can cause some unwanted behaviors. 3) The patch cannot solve all issues caused by SIGALARM handling, because PHP has thousand places which may stay data in inconsistent state (or data allocated in permanent heap). Wrapping all of them with block/unblock is not a decision. small issues: 4) block/unblock code in estrndup() and some other functions is useless. It should be removed. I added that in estrndup() during testing as an interrupt in this function caused String is non-zero terminated warnings. 5) TSRM_FETCH() in zend_hash and zend_alloc functions will slowdown Windows ZTS build even if it doesn't use signals. It may be changed to some other macro that will do TSRM_FETCH only if signals are enabled. This can be done. For zend_alloc, as _emalloc already does TSRMLS_FETCH(), it is possible to pass the tsrm_ls directly as argument to _zend_mm_alloc_int(). _efree, _erealloc, etc do TSRMLS_FETCH() too. There is a ~5% slowdown on bench.php with ZTS builds with the patch applied. And ~3% when passing tsrm_ls to _zend_mm_alloc_int/free/realloc(). Passing the tsrm_ls to hash functions may avoid this slowdown but this requires huge changes. May be if we really like safe SIGALARM handling we may use Windows decision for UNIX too. I mean setup of EG(timed_out) flag in signal handler and checking it in execute loop. As an optimization we can check it not on each instruction but only on enter in function and JMP* opcodes (loops). Anyway, this solution may cause comparable slowdown. It may be a good solution too. Just removing #ifdefs around timed_out (so, without any optimization) gives a 1 to 2% slowdown. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Zend Signal Handling
Hi, I made some changes to the patch: http://arnaud.lb.s3.amazonaws.com/php-5.3.0-alarms-0808141122.patch - Apache effectively seems to resets the signals after MINIT, so original handlers are now saved in RINIT in the first request of the process. - Removed some unneeded blocks/unblocks. - Changed all added TSRMLS_FETCH() so that they affect only ZTS+ZEND_SIGNAL builds. - Removed some added TSRMLS_FETCH() (all in zend_alloc and a few in zend_hash). Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Zend Signal Handling
Hi, On Thursday 14 August 2008 04:19:38 Rasmus Lerdorf wrote: Arnaud Le Blanc wrote: - Apache effectively seems to resets the signals after MINIT, so original handlers are now saved in RINIT in the first request of the process. Did you verify this behaviour with Apache2 as well? I've only checked Apache1. -Rasmus Yes, with the worker MPM it sets at least SIGHUP after MINIT. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] include bug in 5.3
On Saturday 09 August 2008 23:16:30 Antony Dovgal wrote: On 09.08.2008 19:28, Dmitry Stogov wrote: The improved patch fixes all the issues I found during testing. However I wasn't able to test it on NETWARE and on Solaris with relative paths. Please test it as much as possible. I'm going to commit it on Tuesday in case of no objections. ext/standard/tests/file/clearstatcache_001.phpt fails with the patch applied. -- Wbr, Antony Dovgal Hi, The test was wrong, I fixed it. Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] clearstatcache change
On Sunday 10 August 2008 17:14:52 Pierre Joye wrote: hi Arnaud! On Sat, Aug 9, 2008 at 4:20 PM, Arnaud Le Blanc [EMAIL PROTECTED] wrote: Commited :) Do you have a windows dev box? It seems that something is broken on Windows. One reproduce case is ext\standard\tests\file\copy_variation4-win32.phpt which fails using current 5.3-cvs. Let me know if you do not have a win box, I can try to fix it later this week. Cheers, Hi Pierre, No, I do not have a windows box here. Does this test pass without this change ? http://news.php.net/php.cvs/52079 Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] clearstatcache change
On Sunday 10 August 2008 17:46:20 Arnaud Le Blanc wrote: On Sunday 10 August 2008 17:14:52 Pierre Joye wrote: hi Arnaud! On Sat, Aug 9, 2008 at 4:20 PM, Arnaud Le Blanc [EMAIL PROTECTED] wrote: Commited :) Do you have a windows dev box? It seems that something is broken on Windows. One reproduce case is ext\standard\tests\file\copy_variation4-win32.phpt which fails using current 5.3-cvs. Let me know if you do not have a win box, I can try to fix it later this week. Cheers, Hi Pierre, No, I do not have a windows box here. Does this test pass without this change ? http://news.php.net/php.cvs/52079 Regards, Arnaud Just readden the test, actually it seems it just needs to be updated (changing clearstatcache() by clearstatcache(true)). Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] clearstatcache change
On Thursday 07 August 2008 11:33:02 Arnaud Le Blanc wrote: On Thursday 07 August 2008 01:50:06 Johannes Schlüter wrote: On Wed, 2008-08-06 at 21:00 +0200, Arnaud Le Blanc wrote: btw. I just noticed chroot() calls this realpath_cache_clean()..intentional? I'd assume that, as /foo inside a chroot is different from /foo outside... Also some streams stuff uses the php_clear_stat_cache() func but those should propably use the realpatch_cache_del() instead and not blow away whole cache? Yes, I think too. I added that to your patch: http://arnaud.lb.s3.amazonaws.com/clearstatcache_optional_params.patc h :) It also adds the filename argument to clearstatcache([bool clear_realpath_cache[, filename]]) I reply to myself, actually this may cause troubles to not clear the full cache in plain_wrapper.c :/ I updated the patch, just left the filename argument to clearstatcache(). If you fix the arginfo like Hannes it's, good. If the name of the second parameter in the proto (filename) is the same as in the implementation (pathname) it might even be a bit better :-) johanes Commited :) Hi, I have noticed that there is no function to know the realpath cache usage (e.g. is the realpath cache full ? what files are in the cache ?) I have written two functions: realpath_cache_get_entries() which returns an array of entries in the cache, and realpath_cache_get_usage() which returns the cache size. The first one may also help in writing tests for the realpath cache. http://arnaud.lb.s3.amazonaws.com/realpath_cache_funcs.patch Can I add them ? Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Zend Signal Handling
Hi, As Lucas said the patch seems ready now, could someone please review the patch for inclusion ? http://wiki.php.net/rfc/zendsignals Changes that have been made: - The patch has been ported to HEAD - The patch now supports multithreaded environments, and fixes many problems on non-windows platforms when enabled. - Extensions can use zend_signal() and zend_sigaction() so that all signals they declare will always be deferred in critical sections. PCNTL has been ported. - There is now a zend.signal_check ini entry so that the checks made in zend_signal_deactivate() can be done in non-debug builds. - Some code has been moved to process startup or global constructor routines. - The patch moves some HANDLE_BLOCK/UNBLOCK in zend_alloc to protect more code Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] include bug in 5.3
Hi, On Friday 08 August 2008 17:52:26 Dmitry Stogov wrote: Hi, The attached patch is going to fix the problem. It implements its own realpath() function, so we won't depend on system anymore. It also improve realpath cache usage by caching intermediate results. I tested it on Linux and Windows only and it seems to work without problems. It breaks one test related to clearstatcache() function, but this break is expected. Could you please test it. Does it really fix the bug on FreeBSD? Yes, it fixes the bug on FreeBSD :) Thanks. Dmitry. Hannes Magnusson wrote: On Thu, Aug 7, 2008 at 22:37, Arnaud Le Blanc [EMAIL PROTECTED] wrote: virtual_file_ex() assumes that realpath() returns NULL when the file does not exists. But BSD's realpath() will not return NULL if all components *but the last* exist. So realpath(/foo/bar) will return /foo/bar even if bar does not exists. And that behavior is intended, but is sadly overwritten in ZTS :( -Hannes Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] clearstatcache change
On Thursday 07 August 2008 01:50:06 Johannes Schlüter wrote: On Wed, 2008-08-06 at 21:00 +0200, Arnaud Le Blanc wrote: btw. I just noticed chroot() calls this realpath_cache_clean()..intentional? I'd assume that, as /foo inside a chroot is different from /foo outside... Also some streams stuff uses the php_clear_stat_cache() func but those should propably use the realpatch_cache_del() instead and not blow away whole cache? Yes, I think too. I added that to your patch: http://arnaud.lb.s3.amazonaws.com/clearstatcache_optional_params.patch :) It also adds the filename argument to clearstatcache([bool clear_realpath_cache[, filename]]) I reply to myself, actually this may cause troubles to not clear the full cache in plain_wrapper.c :/ I updated the patch, just left the filename argument to clearstatcache(). If you fix the arginfo like Hannes it's, good. If the name of the second parameter in the proto (filename) is the same as in the implementation (pathname) it might even be a bit better :-) johanes Commited :) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] PCRE and recursion: NO_RECURSE flag
Hi, PCRE can be highly recursive [1] and this can cause segmentation faults [2]. This is a known problem and the pcre ini settings avoids the segfaults, but the execution of the expression will fail anyway. PCRE has a NO_RECURSE flag which makes the match() internal function no recursive and avoids this problem. The configure switch for this option is -- disable-stack-for-recursion. NO_RECURSE is documented in pcre_exec.c. This can be easily enabled [3] for the bundled PCRE. It seems to be safe to enable that. Actually the PCRE test suite pass with NO_RECURSE enabled, and segfaults without. Can this be enabled ? [1] http://manpages.courier-mta.org/htmlman3/pcrestack.3.html [2] http://bugs.php.net/bug.php?id=45735 [3] http://arnaud.lb.s3.amazonaws.com/pcre_no_recurse.patch Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PCRE and recursion: NO_RECURSE flag
Sorry, I had not thought about that. I just checked with the example given in the bug report and it's effectively slower (~25% on this expression). On Thursday 07 August 2008 14:35:12 Nuno Lopes wrote: No! Last time I benchmark it, it has about 100x slower, so I don't think so.. If you need to match large amounts of data, just increase the stack size. ulimit is your friend :) Nuno - Original Message - From: Arnaud Le Blanc [EMAIL PROTECTED] To: internals@lists.php.net Sent: Thursday, August 07, 2008 1:24 PM Subject: [PHP-DEV] PCRE and recursion: NO_RECURSE flag Hi, PCRE can be highly recursive [1] and this can cause segmentation faults [2]. This is a known problem and the pcre ini settings avoids the segfaults, but the execution of the expression will fail anyway. PCRE has a NO_RECURSE flag which makes the match() internal function no recursive and avoids this problem. The configure switch for this option is -- disable-stack-for-recursion. NO_RECURSE is documented in pcre_exec.c. This can be easily enabled [3] for the bundled PCRE. It seems to be safe to enable that. Actually the PCRE test suite pass with NO_RECURSE enabled, and segfaults without. Can this be enabled ? [1] http://manpages.courier-mta.org/htmlman3/pcrestack.3.html [2] http://bugs.php.net/bug.php?id=45735 [3] http://arnaud.lb.s3.amazonaws.com/pcre_no_recurse.patch Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PHP-CVS] Re: cvs: php-src /ext/pcntl EXPERIMENTAL
Hi, On Thursday 07 August 2008 16:48:30 Johannes Schlüter wrote: Hi, On Thu, 2008-08-07 at 13:07 +, Antony Dovgal wrote: tony2001Thu Aug 7 13:07:07 2008 UTC Removed files: /php-src/ext/pcntl EXPERIMENTAL Log: remove EXPERIMENTAL flag The EXTENSIONS file says EXTENSION: pcntl MAINTENANCE: Unknown STATUS: Working Now that we're removing the EXPERIMENTAL flag: Anybody who wants to step in? johannes If you mean that a maintainer is needed for pcntl, I'm interested in maintaining it :) Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PHP-CVS] Re: cvs: php-src /ext/pcntl EXPERIMENTAL
On Thursday 07 August 2008 17:01:52 Lukas Kahwe Smith wrote: On 07.08.2008, at 16:59, Arnaud Le Blanc wrote: Hi, On Thursday 07 August 2008 16:48:30 Johannes Schlüter wrote: Hi, On Thu, 2008-08-07 at 13:07 +, Antony Dovgal wrote: tony2001 Thu Aug 7 13:07:07 2008 UTC Removed files: /php-src/ext/pcntl EXPERIMENTAL Log: remove EXPERIMENTAL flag The EXTENSIONS file says EXTENSION: pcntl MAINTENANCE: Unknown STATUS: Working Now that we're removing the EXPERIMENTAL flag: Anybody who wants to step in? johannes If you mean that a maintainer is needed for pcntl, I'm interested in maintaining it :) I was just going to propose you :) regards, Lukas Kahwe Smith [EMAIL PROTECTED] So, I accept :) Regards, Arnaud -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] include bug in 5.3
On Thursday 07 August 2008 21:52:37 Rasmus Lerdorf wrote:
Felipe Pena wrote:
Em Qui, 2008-08-07 às 20:55 +0200, Hannes Magnusson escreveu:
On Thu, Aug 7, 2008 at 20:20, Rasmus Lerdorf[EMAIL PROTECTED] wrote:
Christian Stocker wrote:
Hi
Since quite some time (weeks), I have this very strange and annoying
include bug in 5.3-dev and now I think is the time to report it :)
The reproducable script is here:
http://trash.chregu.tv/include-bug.php.txt
When trying debug it days ago, I noticed add that:
+++ TSRM/tsrm_virtual_cwd.c 4 Jul 2008 03:05:16 -
@@ -557,9 +557,11 @@ CWD_API int virtual_file_ex(cwd_state *s
if (use_realpath != CWD_EXPAND) {
#if !defined(TSRM_WIN32) !defined(NETWARE)
char resolved_path[MAXPATHLEN];
+ printf( %s\n, path);
if (!realpath(path, resolved_path)) { /* Note: Not
threadsafe on older *BSD's */
+ printf( 1\n);
if (use_realpath == CWD_REALPATH) {
+ printf( 2\n);
LINUX:
/home/felipe/php5/sapi/cli/php
/home/felipe/php5/sapi/cli/php
/home/felipe/php5/sapi/cli/php-cli.ini
1
/usr/local/lib/php-cli.ini
1
/home/felipe/php5/sapi/cli/php.ini
1
/usr/local/lib/php.ini
/usr/local/lib/php.ini
/home/felipe/test/test.php
/home/felipe/./foo.php
1
2
/usr/local/lib/php/foo.php
1
2
/home/felipe/test/foo.php
foo
--
BSD:
/usr/home/felipe/php5/sapi/cli/php
/usr/home/felipe/test/test.php
/usr/home/felipe/./foo.php
Warning: include(/usr/home/felipe/foo.php): failed to open stream: No
such file or directory in /usr/home/felipe/test/test.php on line 1
Warning: include(): Failed opening 'foo.php' for inclusion
(include_path='.:/usr/local/lib/php') in /usr/home/felipe/test/test.php
on line 1
That's not the same thing though. Your case seems to show that it isn't
using the include_path at all. On BSD after checking ./foo.php why
doesn't it continue on to /usr/local/lib/php/foo.php? It certain does
for me here.
-Rasmus
Hi,
I had searched the cause of this bug some days ago and posted what I have
found on http://bugs.php.net/bug.php?id=45044 .
virtual_file_ex() assumes that realpath() returns NULL when the file does not
exists. But BSD's realpath() will not return NULL if all
components *but the last* exist. So realpath(/foo/bar) will return /foo/bar
even if bar does not exists.
Regards,
Arnaud
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] clearstatcache change
Hi, On Wednesday 06 August 2008 18:18:49 Jani Taskinen wrote: Rasmus Lerdorf wrote: I think we either need to make clearstatcache() not affect the realpath cache, or we should add an optional argument to it to specify whether or not the realpath cache should be cleared as well. See this: http://bugs.php.net/39367 Considering some people seem to want to clear realpath cache as well, I'd make it an optional parameter which defaults to NOT clear realpath cache: clearstatchache(true); /* BOOM! realpath cache gone.. */ btw. I just noticed chroot() calls this realpath_cache_clean()..intentional? Also some streams stuff uses the php_clear_stat_cache() func but those should propably use the realpatch_cache_del() instead and not blow away whole cache? Yes, I think too. I added that to your patch: http://arnaud.lb.s3.amazonaws.com/clearstatcache_optional_params.patch :) It also adds the filename argument to clearstatcache([bool clear_realpath_cache[, filename]]) Here is a patch to add that optional parameter (for PHP_5_3): http://pecl.php.net/~jani/patches/clearstatcache_optional_param.patch --Jani -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
