{"action":"created","issue":{"url":"https://api.github.com/repos/nette/tracy/issues/223","repository_url":"https://api.github.com/repos/nette/tracy","labels_url":"https://api.github.com/repos/nette/tracy/issues/223/labels{/name}","comments_url":"https://api.github.com/repos/nette/tracy/issues/223/comments","events_url":"https://api.github.com/repos/nette/tracy/issues/223/events","html_url":"https://github.com/nette/tracy/pull/223","id":177639855,"number":223,"title":"optimize Dumper::encodeString for long strings","user":{"login":"schlndh","id":12036937,"avatar_url":"https://avatars.githubusercontent.com/u/12036937?v=3","gravatar_id":"","url":"https://api.github.com/users/schlndh","html_url":"https://github.com/schlndh","followers_url":"https://api.github.com/users/schlndh/followers","following_url":"https://api.github.com/users/schlndh/following{/other_user}","gists_url":"https://api.github.com/users/schlndh/gists{/gist_id}","starred_url":"https://api.github.com/users/schlndh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schlndh/subscriptions","organizations_url":"https://api.github.com/users/schlndh/orgs","repos_url":"https://api.github.com/users/schlndh/repos","events_url":"https://api.github.com/users/schlndh/events{/privacy}","received_events_url":"https://api.github.com/users/schlndh/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-09-18T08:54:54Z","updated_at":"2016-09-18T13:12:25Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/nette/tracy/pulls/223","html_url":"https://github.com/nette/tracy/pull/223","diff_url":"https://github.com/nette/tracy/pull/223.diff","patch_url":"https://github.com/nette/tracy/pull/223.patch"},"body":"- feature\r\n- issues - none\r\n- documentation - not needed\r\n- BC break - yes\r\n\r\nThe purpose of this PR is to speedup dumping of large strings for reasonable maxLength. The performance problem is in `preg_match('#[^\\x09\\x0A\\x0D\\x20-\\x7E\\xA0-\\x{10FFFF}]#u', $s)` which scans the whole string. By preferring `mb_substr` it is possible to truncate the string before running it through the regex and thus potentially saving a lot of time for large strings. However this is a slight BC break, because now it won't recognize some binary strings it did recognize before, but since the binary part will be outside the truncated string I think it is worth it.\r\n\r\nAs you can see from [the benchmark I made](https://gist.github.com/schlndh/e7f3e9c7f98ce8131ef3452b8bb46fb5) the performance impact for short strings or large strings with unlimited maxLength is minimal or none, however the performance gain for long strings with small maxLength is massive.\r\n\r\nThe benchmark compares current `encodeString` implementation (encodeStringOrig) with implementation from this PR (encodeStringChanged) and just running the `preg_match` on the whole string. The first column (N) is the number of characters and the values of other columns are average times for one call on given string in nanoseconds.\r\n\r\nBenchmarking was done on Linux with PHP 7.0.10\r\n"},"comment":{"url":"https://api.github.com/repos/nette/tracy/issues/comments/247846934","html_url":"https://github.com/nette/tracy/pull/223#issuecomment-247846934","issue_url":"https://api.github.com/repos/nette/tracy/issues/223","id":247846934,"user":{"login":"schlndh","id":12036937,"avatar_url":"https://avatars.githubusercontent.com/u/12036937?v=3","gravatar_id":"","url":"https://api.github.com/users/schlndh","html_url":"https://github.com/schlndh","followers_url":"https://api.github.com/users/schlndh/followers","following_url":"https://api.github.com/users/schlndh/following{/other_user}","gists_url":"https://api.github.com/users/schlndh/gists{/gist_id}","starred_url":"https://api.github.com/users/schlndh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schlndh/subscriptions","organizations_url":"https://api.github.com/users/schlndh/orgs","repos_url":"https://api.github.com/users/schlndh/repos","events_url":"https://api.github.com/users/schlndh/events{/privacy}","received_events_url":"https://api.github.com/users/schlndh/received_events","type":"User","site_admin":false},"created_at":"2016-09-18T13:12:25Z","updated_at":"2016-09-18T13:12:25Z","body":"@JanTvrdik Thanks for your comments. It didn't think of checking `mb_substr` with binary strings before, but it sort of works:\r\n```php\r\n$lengths = [];\r\nfor ($i < 0; $i < 1000; ++$i) {\r\n\t$s = openssl_random_pseudo_bytes(1000);\r\n\t@$lengths[strlen(mb_substr($s, 0, 150, 'UTF-8'))]++;\r\n}\r\nksort($lengths);\r\nforeach ($lengths as $k => $val) {\r\n\techo $k . \" \" . str_repeat('|', $val) . \"\\n\";\r\n}\r\n```\r\nIt makes the substring mostly around 200 bytes long, which is fine I guess."}}
{"actor":{"display_login":"schlndh"}}