{"action":"created","issue":{"url":"https://api.github.com/repos/netty/netty/issues/6790","repository_url":"https://api.github.com/repos/netty/netty","labels_url":"https://api.github.com/repos/netty/netty/issues/6790/labels{/name}","comments_url":"https://api.github.com/repos/netty/netty/issues/6790/comments","events_url":"https://api.github.com/repos/netty/netty/issues/6790/events","html_url":"https://github.com/netty/netty/pull/6790","id":232225949,"number":6790,"title":"Improvement : \"if\" construction replaced with \"switch\" for ServerCookieDecoder","user":{"login":"doom369","id":1536494,"avatar_url":"https://avatars1.githubusercontent.com/u/1536494?v=3","gravatar_id":"","url":"https://api.github.com/users/doom369","html_url":"https://github.com/doom369","followers_url":"https://api.github.com/users/doom369/followers","following_url":"https://api.github.com/users/doom369/following{/other_user}","gists_url":"https://api.github.com/users/doom369/gists{/gist_id}","starred_url":"https://api.github.com/users/doom369/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/doom369/subscriptions","organizations_url":"https://api.github.com/users/doom369/orgs","repos_url":"https://api.github.com/users/doom369/repos","events_url":"https://api.github.com/users/doom369/events{/privacy}","received_events_url":"https://api.github.com/users/doom369/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2017-05-30T11:45:17Z","updated_at":"2017-05-30T19:36:26Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/netty/netty/pulls/6790","html_url":"https://github.com/netty/netty/pull/6790","diff_url":"https://github.com/netty/netty/pull/6790.diff","patch_url":"https://github.com/netty/netty/pull/6790.patch"},"body":"Motivation:\r\n\r\nUsually, cookies don't start with special chars, so `Switch` statement is more efficient for cookie decoder as it performs less operations.\r\n\r\nModification:\r\n\r\n`If` statement replaced with `switch`. Added 2 more tests for corner cases.\r\n\r\nResult:\r\n\r\nCookie decoding is a bit faster."},"comment":{"url":"https://api.github.com/repos/netty/netty/issues/comments/304984934","html_url":"https://github.com/netty/netty/pull/6790#issuecomment-304984934","issue_url":"https://api.github.com/repos/netty/netty/issues/6790","id":304984934,"user":{"login":"fenik17","id":20779569,"avatar_url":"https://avatars0.githubusercontent.com/u/20779569?v=3","gravatar_id":"","url":"https://api.github.com/users/fenik17","html_url":"https://github.com/fenik17","followers_url":"https://api.github.com/users/fenik17/followers","following_url":"https://api.github.com/users/fenik17/following{/other_user}","gists_url":"https://api.github.com/users/fenik17/gists{/gist_id}","starred_url":"https://api.github.com/users/fenik17/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fenik17/subscriptions","organizations_url":"https://api.github.com/users/fenik17/orgs","repos_url":"https://api.github.com/users/fenik17/repos","events_url":"https://api.github.com/users/fenik17/events{/privacy}","received_events_url":"https://api.github.com/users/fenik17/received_events","type":"User","site_admin":false},"created_at":"2017-05-30T19:36:26Z","updated_at":"2017-05-30T19:36:26Z","body":"> no need to guess. We have JMH for that :).\r\n\r\nnp )\r\n```java\r\nBenchmark                     Mode  Cnt       Score      Error  Units\r\nBitSetBenchmark.testBitMask  thrpt   20  447646,355 ± 4455,793  ops/s\r\nBitSetBenchmark.testBitSet   thrpt   20  369771,190 ± 2982,951  ops/s\r\nBitSetBenchmark.testIf       thrpt   20  339016,457 ± 2166,366  ops/s\r\nBitSetBenchmark.testSwitch   thrpt   20  372332,634 ± 4399,075  ops/s\r\n```\r\nThese results are still within the margin of error, in other test cases the picture can be different. I agree with @slandelle: `switch`s variant can be accepted as more readable.\r\n\r\n<details>\r\n\r\n<summary>Benchmarks code</summary>\r\n\r\n```java\r\n@BenchmarkMode(Mode.Throughput)\r\n@Fork(1)\r\n@State(Scope.Thread)\r\n@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS, batchSize = 1000)\r\n@Measurement(iterations = 20, time = 1, timeUnit = TimeUnit.SECONDS, batchSize = 1000)\r\npublic class BitSetBenchmark extends AbstractMicrobenchmark {\r\n\r\n    private static final BitSet bitSet = new BitSet(64);\r\n    private static final long bitMask = setBits(0, '\\t', '\\n', 0x0b, '\\f', '\\r', ' ', ',', ';');\r\n\r\n    static {\r\n        for (char c : new char[] { '\\t', '\\n', 0x0b, '\\f', '\\r', ' ', ',', ';' }) {\r\n            bitSet.set(c);\r\n        }\r\n    }\r\n\r\n    private static long setBits(long mask, int... bitPos) {\r\n        for (int pos : bitPos) {\r\n            assert pos < 64 && pos >= 0;\r\n            mask |= (1L << pos);\r\n        }\r\n        return mask;\r\n    }\r\n\r\n    private static boolean hasBit(long mask, int bitPos) {\r\n        return bitPos < 64 && (mask & (1L << bitPos)) != 0;\r\n    }\r\n\r\n    private char c = 'm';\r\n\r\n    @Setup\r\n    public void init() {\r\n        c = 'm';\r\n    }\r\n\r\n    @Benchmark\r\n    public boolean testSwitch() {\r\n        switch (c) {\r\n        case '\\t':\r\n        case '\\n':\r\n        case 0x0b:\r\n        case '\\f':\r\n        case '\\r':\r\n        case ' ':\r\n        case ',':\r\n        case ';':\r\n            return true;\r\n        }\r\n        return false;\r\n    }\r\n\r\n    @Benchmark\r\n    public boolean testIf() {\r\n        return c == '\\t' || c == '\\n' || c == 0x0b || c == '\\f'\r\n               || c == '\\r' || c == ' ' || c == ',' || c == ';';\r\n    }\r\n\r\n    @Benchmark\r\n    public boolean testBitSet() {\r\n        return bitSet.get(c);\r\n    }\r\n\r\n    @Benchmark\r\n    public boolean testBitMask() {\r\n        return hasBit(bitMask, c);\r\n    }\r\n}\r\n```\r\n\r\n</details>"}}
{"actor":{"display_login":"fenik17"}}