Re: Improved zend_string API

> Am 01.07.2015 um 18:32 schrieb François Laupretre <francois@php.net>: > >> De : Bob Weinand [mailto:bobwei9@hotmail.com] >> >> Interesting idea (from the concept). I even could get on board with this; I'm >> just not that sure about the performance impact. (alignment of refcount to >> base address etc.) > > zend_string *s; > > s = zend_string_alloc(256, 0); > ... > ZSTR_SET_LEN(s, snprint(s, ZSTR_LEN(s)+1, "<format>", args...); /* > Overflow-protected */ > ... > hash = ZSTR_HASH(s); > ... > char *p = estrndup(s, ZSTR_LEN(s)); /* Result is a pure 'char *', not a zend_string > */ > ... > ZSTR_RELEASE(s); > > Isn't it nice ? > > You're right, refcount alignement is the serious issue to solve. Peformance needs to be > tested of course, but we compute '->val' much more often than we use the struct base > address (for realloc/free only except when accessing gc). Accessing length and hash using an offset > has the same cost as before. > >> Also, you have to make sure, to not accidentally pass a real char * array to >> something that expects a char * from a zend_string *… which is where I'm >> not so sure if we can do it. It may loose some typing security :-/ > > The zend_string type would remain, even if typedef-ed to 'char'. So, > 'zend_string *' declarations would remain, they wouldn't become 'char *' > (just a question of declarations because it would work too). In debug mode, we also can add a marker > in the structure to detect when we receive an invalid address, as it is sometimes done in memory > management libs. As said, if zend_string is typedef'ed to char, we'll end up loosing type safety, because no compiler will warn when we pass a char * to a zend_string * then. The marker would work, though. I'm not sure to what degree it will confuse people. It's not a bad idea, but we need to carefully evaluate it. (P.s.: I'd honestly prefer ZSTR_LEN(s) = ...; instead of ZSTR_SET_LEN(s, ...);) But still, when we do that, API will break again. >> Also wrappers around such low-level structures often tend to be leaky or be >> just such a shitload of individual functions that nobody ever can remember it >> before having worked with it all day for a few months. >> (I mean leaky in a sense that you maybe are able to do everything, but not >> quite in the performant way we like to) > > I agree, that's often the case but, even when performance impose low-level control, an > appropriate abstraction layer often allows for a cleaner future. What I fear most is not this, > it's having an idea impossible to implement because people designing the API did not think wide > enough when it was time. The PHP history is full of such changes which required a lot of time and > energy, just because everyone had been working during years with too short-sighted APIs. The art of > APIs id to find the best compromise between usability, performance, and extensibility, knowing > there's no ideal solution. And one additional difficulty is that many people think it is easy ! There's always something you'll have missed in retrospect. Perfect forward compatibility is impossible. One might think it could be nice, but it ends up not covering everything. >> Meh, that persistent/non-persistent is annoying me a bit too sometimes. >> I don't disagree that we should change that. We could e.g. use GC_TYPE() >> and add our custom flag here. But maybe it's there to be able to tell compiler >> what branch in perealloc() will be taken, so that that branch can be compiled >> out. >> Abstraction APIs are a powerful tool, but they come at a price… > > Actually, the information is already stored in the struct, there's nothing to add. > zend_string_release/free() functions even use it to determine which kind of mem they are freeing. It > is not even consistent because only realloc/extend/truncate use this 'useless' arg. > Actually, the only reason given is too optimize compilation. I know that everything has a price but > I think that's going too much on the performance side. An argument with a single value allowed > is not an argument, IMO. Remember, if the wrong value is given, your program crashes ! Anyway, > I'd first like to measure the performance gains/loss of such choices. You're right. Yeah, just measure first ;-) >> But issue is a bit now that with 7.1 we don't want to do major API changes. >> We're *allowed* to, but we shouldn't. > > While the politics of BC breaks at the PHP level is now quite well-defined, it is less clear at > the C level. During 5.x, we saw several macros and functions disappear without notice, and nobody > seems so surprised about it. IMO, the hardest part is to get a consensus on such changes. Which is a good thing. That leaves us the possibility to decide on a case-by-case basis whether it's worth it. We almost certainly need each version some API breaks to integrate our changes. >> I don't feel controlled by Zend. The only major appearance from Zend was in >> scalar types discussion in form of Zeev. Err yeah, and maybe the PHP 7 name. >> Hah. >> Yes, it's maybe Zend which pushed Dmitry to optimize PHP to death, I have >> no idea. But that's a good thing, stop ranting against that :-P > > I don't say I'm controlled by Zend. I say we shouldn't have given the phpng > whitecard with so few supervision and control from the community. IMHO phpng was the best thing which could happen. And I feel a lot like the most active engine core contributors all were well involved here. Bob