Hacker News new | past | comments | ask | show | jobs | submit login
Improve PHP Disk I/O & remove Race conditions by using '@' error suppression (rodneyrehm.de)
32 points by willmacdonald on Jan 10, 2013 | hide | past | web | favorite | 54 comments



I'm going to completely ignore the ridiculousness of the author's claims and stick with talking about @ suppression for a moment...

Not this again! I've been working with PHP for over 10 years and seen @ suppression used at companies in the most ridiculous ways imaginable. The most common, and I believe incorrect, way @ suppression is used is when using fopen. Why do people do it so often? Probably because it says you can do so right in the php.net manual http://us2.php.net/manual/en/function.fopen.php and has so for years. I've really lost track of how many bugs I've come across over the years in production code that could be traced back to someone @ suppressing fopen warnings. I don't care if you read some blog where the author claims using @ suppression will give you a magical 1000% increase in performance. JUST DON'T DO IT.


Another common misuse I see is suppressing notices for non-existent array elements, e.g. @$array['key'].


Another common misuse I see is suppressing notices for non-existent array elements, e.g. @$array['key']

Oh wow, that's impressively awful, way more evil than the other common "just turn off notices in php.ini" hack.


I don't think that's intrinsically bad. Sometimes you want getting a non-existent key to noisily break, sometimes returning null is a-okay. Many languages have maps/dictionaries with a non-throwing get() operation.


it is really horrible, just use isset().


I've being using this trick for years instead of the ugly

    isset($array['key']) ? $array['key'] : null;
thinking I was simply ignoring those stupid "Undefined index" notices.

Now I figure out I was also hiding "Undefined variable" notices when $array was not defined and a ton of potential errors when $array is an object implementing ArrayAccess.

Shame on me.


Finally, an explanation for why it's a bad idea, after all these people just going "it's a misuse", "impressively awful", "really horrible"...

Thanks.

Note to self: grep the code at work for @. Purge.


Yes, the main issue with using @ is that it conceals side effects and sweeps all errors under the rug -- including the ones that you may actually want to handle.

Although there may be some scenarios where it's permissible, such as @unlinking temporary files when upload validation fails. It only happens in exceptional cases, the outcome is not immediately crucial, and you don't have any dependent operations.

All of this is moot when you do the following though:

- Set error_reporting to -1 (report ALL errors)

- Register a custom error handler that converts all errors to exceptions

This renders the @ operator useless (it'll throw an exception regardless). I personally prefer this since it enforces error handling discipline early on.


That's an interesting idea on the custom error handler, I wasn't aware that those trigger regardless of @s.

I guess a lot of the trouble with @ is that it affects everything that happens temporally-within the expression, but people often use it as if it only affects everything syntactically-within it... for instance, recently I came across something like:

    // suppress warning for <2 results
    @list ($foo, $bar) = returns_a_list(...);
... which would also hide any warnings that occur in the body of returns_a_list(), despite that being neither intended nor wanted.

It's a shame the only @-free solutions to many popular uses of @ (like that one) are so hideously ugly. Also that I suspect that changing @ so it only affected syntactically-contained errors would be... impractical for the PHP team, for a number of reasons.


I have seen @ suppression on includes, which is probably the worst abuse I have seen.


Yep, seen @include_once("filethatdoesallxssfiltering.php") before.


There should be a big fat warning in front of this very bad advice, and I particularly dislike the last paragraph about intercepting error handlers to cover up your ugly hack.Some newcomer is going to run with this idea without understanding the many, many problems it can introduce. Until 5.4 the @ operator caused dramatic slowdowns on everything it touched because PHP had to jump through hoops to suppress the errors. It's funny to see someone advocating using it for performance reasons now. If you need a mutex, you should use a mutex.

Yet another reason not to use smarty I guess.


Define "dramatic slowdowns" ?

added: Apparently someone on php.net agrees with you

http://php.net/manual/en/language.operators.errorcontrol.php...

they say it takes 1.7 times as long BUT that is only .005 ms in their example


That example is amongst the worst synthetic benchmarks I have seen in a while. Also it comes from someone who is associated with piwik. A PHP project that makes wordpress look tidy, well engineered and speedy.

The more deeply nested the callstack is when you supress, the bigger the performance hit (slight generalisation and slightly from memory).


I spent 10-15 minutes poking around php internals newslist for an explanation of the php 5.4 changelog "improved performance of the (@) silence operator" without luck. It's just listed as a zend engine improvement without specifications.

Someone posted here a clear example of why (in earlier php5) it generates far worse opcode:

http://derickrethans.nl/five-reasons-why-the-shutop-operator...


I'm being offered a "Serbo-Croatian" translation and the site's captcha is broken. Might be unwise to judge authority on the subject just from that, however I decided not to read the article.


You are correct. It is wrong -very very wrong- to view programming advice as suspect merely because the content is offered in a language you don't speak.

EDIT: I am wrong. See below.


It's being offered in a language that does not exist. http://en.wikipedia.org/wiki/List_of_ISO_639-1_codes


It's being offered in a language that does not exist. http://en.wikipedia.org/wiki/List_of_ISO_639-1_codes

Wikipedia has what appears to be quite a comprehensive article about it:

http://en.wikipedia.org/wiki/Serbo-Croatian

Its scope is referred to as a "macrolanguage" here under ISO 639-3:

http://www.sil.org/iso639-3/documentation.asp?id=hbs


And this is another reason why everyone regards PHP developers as being a tinsy bit crap at developing software. It's a broken solution to a problem that shouldn't really be there. Ignoring why the race condition is happening, what is wrong with something like Redis or heaven forbid semaphores/shared memory for locking. There are billions of ways of doing atomic updates right and this is not one of them

--EDIT

I hadn't even got as far as the disk i/o section, after reading no words can do justice to the complete lack of understanding shown here with regards to filesystems, caching, SSD's and linux.


And this is another reason why everyone regards PHP developers as being a tinsy bit crap at developing software. It's a broken solution to a problem that shouldn't really be there.

Very much agree with you.

When I was first starting out programming commercially, one of the best bits of advice I received was something along the lines of "Take a step back and think about why you're using these crazy constructs - chances are there's a much simpler way of doing things."

The lesson being: if you're having to write code like this, chances are it's because you're creating a mess. Take a step back and work out a simpler solution.


Not to mention you wouldn't even have a need for this so-called "optimization" in production, since you'd have your templates precompiled and cached (and turned the compile check off).


Yeah, but you get these somewhat popular outdated libraries pop up and people point at them and say "this is shit!". It is.

There are better ways to do PHP... and there is bad PHP code examples and also good PHP code examples like there is in any language.


Very valid point, whilst some evils are necessary, I think it might be responsible of the author to point out the greater context of what he is doing and why so people don't start doing this willy-nilly.

As a contrived example, an article about "How I replaced my engine using a crowbar" might lead people to believe that is the ideal way to replace engines and have mechanics groaning everywhere. If however the article was "How I replaced my engine using a crowbar whilst stuck in the desert with no other tools to hand" then everyone would understand a) don't do this in normal circumstance, b) there is another way to do it in less normal circumstances.


"And this is another reason why everyone regards PHP developers as being a tinsy bit crap at developing software."

Replace "PHP" with "Jewish" and would you find that comment of yours acceptable?

Edit: I can only assume that the down voters find discrimination acceptable behavior in this community.


Trollolol much? The religion of the author (is he even jewish? - I have no idea nor really care) seems fairly irrelevant here. In the context of the article the author is acting as a PHP developer. Therefore his actions will viewed as such and any opinions will on his work will (I assume) most likely become referenced to 'PHP developers'. If there was a seemingly large correlation with being jewish and producing crap software, and this article just made that correlation stronger, then yes I would make that comment.


You missed my point, big time. It was your sweeping comment lumping all PHP developers as being regarded as crap developers I was getting at. You wouldn't make comments like that about any other groups of people and find it acceptable .. or do you find discrimination acceptable?


I am a PHP developer (amongst other things), and incase you hadn't noticed we are frequently regarded by developers of other languages as being at the comedy/clown end of the development spectrum too often. I didn't say all PHP developers are crap (although I think there are lots that are beyond dire), i said we are observed as being crap, to paraphrase. I would happily make comments about any group based upon observation or information I felt to be true. Furthermore - there is no discrimination in my post that I'm aware of. Even if there was, discrimination by itself is not a 'good' or 'evil' thing, it is what that discrimination is then applied to or for that introduces any ethical/moral dimension.


I wasn't aware you were a PHP developer and were making the comment from the point of view as one; something you might have wanted to mention really with an implication such as the one I took issue with.

Either way, I stand by my comments.


Maybe I didn't understand this article properly, but is it suggesting suppressing ALL filesystem related errors rather than handling them properly?

OK I get that PHP doesn't support atomic operations, but there are so many different errors that could happen when working with files. Say a permissions error, if the permissions of the file were changed between it being created and deleted that's an issue that needs resolving before your tmp folder fills up, it's easy enough to fix and should be reported as an error (be it to the front-end or to a backend logging service).

Surely there is a better way to handle these race conditions without just ignoring all filesystem related errors.


No, I think the author knows the issues behind this muting - you just need to be smart about how you apply it. In the case of filemtime, it appears the only failure condition is a file not existing so we can safely ignore the E_WARNING it emits; it wouldn't be sensible to apply this everywhere (I hadn't thought about the permissions case myself).

The best/real solution is probably implementing flock so that it can issue real filesystem locks, rather than relying on a file resource.


And people wonder why serious PHP developers (yes, those exist) avoid Smarty like the plague...


Understanding why smarty is a bad thing is one of the first steps to PHP enlightenment.


Could you link to some good material about why smarty should be avoided? I'm eager to read about it.

We're using smarty at work now, and while we probably won't drop it in this project (too much working code written in it), we might learn something for the future.


I have not used PHP for a long time. But one major reason to avoid Smarty is that it is damn slow.

I believe Twig is a fast and secure PHP template language.


I was just recently evaluating Twig for a project.

I'm used to Jinja2 in Python which is almost identical to Twig.

Has anyone used Twig who would share their experience?


It'd be better if you told us why you are using Smarty in the first place. I suspect a lot of places use it simply because they've heard a lot of other places use it.


I don't know why. When I came to the project, it was already there - the company has its own framework.

My guess is that whoever wrote the framework just heard about Smarty while in university, long time ago.


My company also uses Smarty, but we're planning to move away from it. (Currently evaluating Mustache.php)

Smarty's parser/lexer is horribly slow. The 3.1.x branch has caused problems for us, such as corrupted compiles and broken nested blocks [1]. As evidenced by the article, the developers don't seem to know what they're doing. You should see the workaround for people with custom error handlers -- registering another error handler on top that checks the source of the error against registered template and cache directory paths!

[1] The bug was introduced in this particular commit (lines 237-241 of r4505): https://code.google.com/p/smarty-php/source/diff?spec=svn450... and later "fixed" here: https://code.google.com/p/smarty-php/source/diff?spec=svn456...


sounds about right. I don't think anyone would choose Smarty for actual technical reasons at this point. I personally think plain PHP makes a far superior templating system. Smarty is slow, doesn't "protect" designers from any complexity because it's complex itself and is yet another thing to learn.


My guess is that is the only reason.

Otherwise PHP itself is a template language. That is why you start it with <? tag.

Just keep short tags on [it's off by default on new installations but ON by default on all hosting sites] and you got a better template engine than smarty.


PHP does not support HTML escaping and is therefor not secure by default. At least twig escapes HTML by default (I am not up to date with PHP so the others might too).

You do not want to type <?php echo htmlspecialchars($var, ENT_QUOTES) ?> every time you want to output data. (Yes, I know it could probably be written shorter but my PHP is rusty. My point still remains though, you have to remember to type it every time.)


I think my solution back in the day was just to include the template files by running a function, something like showTemplate($templateName, $templateVars). The function takes an array as an arg so only these values are available to the template context (apart from the many global vars of course).

You can then run this entire array through htmlentities or htmlspecialchars before doing include().


this is exactly how many modern frameworks do it. your response body is assembled and cached as your application executes, along with any variables it needs, and before the template is rendered the variables are sanitized.


I think it's good this way, it forces the developer to think about the implications of escaped vs unescaped output. The way I see it, having htmlspecialchars/htmlentities applied automatically by a template engine is a close relative to the magic quotes; it abstracts something esential for the developer to know.


For those considering Twig or other templating alternatives: http://phptemplatinglanguage.com/

Really, PHP itself is a fine templating language if used properly. Get to know the short tags (including the short echo <?=$stuff?>) and the alternative syntax for conditional statements and you are good to go.


He exaggerates the costs of running stat. Stat should not cause any disk IO in the hot case and is therefore quite cheap. Especially since he already need to hit the file system anyway to delete the file.

I think most of the work here will happen in memory.


Apart from all of the other considerations outlined here we also have a claim of increased disk performance without any actual evidence that this is the case.

Surely file_exists() is simply an operation on the disk index where the relevant block will be sitting in RAM from the first point that the operation is done. Subsequent operations on the same data shouldn't involve hitting the disk again until an actual write has occurred. Provided you have sufficient RAM this sort of thing is what kernel page cache is designed for.

Not to mention that thinking about a frequently used performance critical piece of PHP code doing large amounts of @fopen() and @unlink() fills me with dread.


I don't understand all the hate this article is generating, it's just a basic case of the 'easier to ask forgiveness than permission' vs 'look before you leap' styles.

PHP warnings are a debugging tool, not a substitute for exceptions. Suppressing them is not the equivalent of a catching and ignoring an exception.

Putting locks everywhere in a futile attempt to prevent the filesystem from changing under you is futile. It's fundamentally an external i/o device out of your program's control. It is never safe or reasonable to infer from the result of one filesystem operation that a subsequent one will not fail.


The title makes me cry!


"Improve VB uptime & remove error messages by using 'On Error Resume Next'!"


The first paragraph alone hurts my brain.

process switching =/= parallel execution multi-threading =/= multi-tasking

Only certain single core processors can execute in parallel, so to say that it's possible to execute in parallel on a single core without specifying that not ALL single core CPUs can do it is bad form.


If you're on top of Linux I'd suggest as a working alterative without the hackery: http://uk3.php.net/manual/en/book.sem.php


Forgive me for (possibly) being naive, but why wouldn't you use structured error handing here? PHP's exceptions are great and very extensible.




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact

Search: