{"action":"created","issue":{"url":"https://api.github.com/repos/jitsi/ice4j/issues/157","repository_url":"https://api.github.com/repos/jitsi/ice4j","labels_url":"https://api.github.com/repos/jitsi/ice4j/issues/157/labels{/name}","comments_url":"https://api.github.com/repos/jitsi/ice4j/issues/157/comments","events_url":"https://api.github.com/repos/jitsi/ice4j/issues/157/events","html_url":"https://github.com/jitsi/ice4j/pull/157","id":379512877,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI5OTQ1NTY2","number":157,"title":"Do not explicitly create threads in PacketQueue","user":{"login":"mstyura","id":385639,"node_id":"MDQ6VXNlcjM4NTYzOQ==","avatar_url":"https://avatars1.githubusercontent.com/u/385639?v=4","gravatar_id":"","url":"https://api.github.com/users/mstyura","html_url":"https://github.com/mstyura","followers_url":"https://api.github.com/users/mstyura/followers","following_url":"https://api.github.com/users/mstyura/following{/other_user}","gists_url":"https://api.github.com/users/mstyura/gists{/gist_id}","starred_url":"https://api.github.com/users/mstyura/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mstyura/subscriptions","organizations_url":"https://api.github.com/users/mstyura/orgs","repos_url":"https://api.github.com/users/mstyura/repos","events_url":"https://api.github.com/users/mstyura/events{/privacy}","received_events_url":"https://api.github.com/users/mstyura/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":26,"created_at":"2018-11-11T11:16:52Z","updated_at":"2018-12-13T09:53:59Z","closed_at":"2018-12-12T16:37:29Z","author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/jitsi/ice4j/pulls/157","html_url":"https://github.com/jitsi/ice4j/pull/157","diff_url":"https://github.com/jitsi/ice4j/pull/157.diff","patch_url":"https://github.com/jitsi/ice4j/pull/157.patch"},"body":"Do not explicitly create threads in `PacketQueue`, but borrow threads for execution from `ExecutorService`.\r\n\r\nCurrently there is one inheritor from `ice4j`'s `PacketQuque<T>` which is [`RawPacketQueue`](https://github.com/jitsi/libjitsi/blob/6c0a31d6193dcd2b012ae3ced656ad403403d9ff/src/org/jitsi/util/RawPacketQueue.java#L21) in `libjitsi` used by [`SctpConnection`](https://github.com/jitsi/jitsi-videobridge/blob/56d941ae8634a2f2d781f00a7d1a259ee7d1d9d6/src/main/java/org/jitsi/videobridge/SctpConnection.java#L255) in videobridge.\r\n\r\nIt is observed that currently there is `N` threads created to process queues created by `SctpConnection`. `N` is the total number of peers connected to `JVB`. Most of the time threads are idle and blocked in `wait(100)` inside `while(true)` loop inside empty `PacketQueue`.\r\n\r\nThis pull request enhances implementation of `PacketQueue` which now temporary borrows execution thread from shared `ExecutorService` and return thread back to pool when there is no work, this results in less threads used and better usage of existing threads managed by `ExecutorService`.\r\n\r\nAs comment at the header of `PacketQueue` states:\r\n```/**\r\n * An abstract queue of packets. This is meant to eventually be able to be used\r\n * in the following classes (in ice4j and libjitsi) in place of their ad-hoc\r\n * queue implementations (which is the reason the class is parameterized):\r\n *     <br> RTPConnectorOutputStream.Queue\r\n *     <br> PushSourceStreamImpl#readQ\r\n *     <br> OutputDataStreamImpl#writeQ\r\n *     <br> SinglePortHarvester.MySocket#queue\r\n *     <br> MultiplexingSocket#received (and the rest of Multiplex* classes).\r\n *\r\n * @author Boris Grozev\r\n */ \r\n```\r\n\r\nThis `PacketQueue` implementation could also dramatically reduce thread usage and enhance overall system performance when aforementioned classes migrated from ad-hoc queue implementations to `PacketQueue`.\r\n\r\nCurrent implementation allow switching between different `ExecutorService` implementations: depending on actual handler work, different `ExecutorService` implementation might give different overall performance.\r\n\r\nSummary:\r\n\r\n* Implementation temporrary borrow execution thread from configurable `ExecutorService` instead of explicit thread creation and waiting when no items in queue to proceed.\r\n* Throttling functionality was added to satisfy requirements of future move of [`RTPConnectorOutputStream.Queue`](https://github.com/jitsi/libjitsi/blob/6c0a31d6193dcd2b012ae3ced656ad403403d9ff/src/org/jitsi/impl/neomedia/RTPConnectorOutputStream.java#L726) to `PacketQueue`\r\n* Due to `ExecutorService` is usually shared between many queues the support of cooperative multi-tasking was added (queue temporary stops it's processing to give execution time to other queues which might work on same `ExecutorService`)\r\n* Unit tests of `PacketQueue` was added\r\n* [Benchmark](https://github.com/jitsi/ice4j/pull/157/files#diff-1519b8e3ad85071f4ff7c1f77cbdde58R14) to compare different `ExecutorService` performance was added. The comparison with baseline implementation added as en example to [different branch in my fork](https://github.com/mstyura/ice4j/commit/37c28b2ae33dbd940d57e581172e3c36f9530585#diff-1519b8e3ad85071f4ff7c1f77cbdde58R134).\r\n\r\nSome of micro-benchmark results on my machine (there are too many variations of `numberOfQueues`/`numberOfItemsInQueue`/`singleQueueItemProcessingWeight` to provide all of them):\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|[iterations](https://github.com/jitsi/ice4j/blob/635d8b98d3cb8d5405dce7a434eb8103727a8ccb/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L24) | 100\r\n|[numberOfQueues](https://github.com/jitsi/ice4j/blob/e466aa49f1009ed00981924fbbd33c5156704571/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L30) | **1200**\r\n|[numberOfItemsInQueue](https://github.com/jitsi/ice4j/blob/e466aa49f1009ed00981924fbbd33c5156704571/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L37) | 1000\r\n|[singleQueueItemProcessingWeight](https://github.com/jitsi/ice4j/blob/e466aa49f1009ed00981924fbbd33c5156704571/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L42) | 300\r\n|[ThreadPerQueuePool](https://github.com/jitsi/ice4j/blob/635d8b98d3cb8d5405dce7a434eb8103727a8ccb/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L56)| avg = 1371 ms, std_dev = 1 ms\r\n|[CachedThreadPerQueuePool](https://github.com/jitsi/ice4j/blob/635d8b98d3cb8d5405dce7a434eb8103727a8ccb/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L77)| avg = 1271 ms, std_dev = 0 ms\r\n|[FixedSizeCPUBoundPool](https://github.com/jitsi/ice4j/blob/635d8b98d3cb8d5405dce7a434eb8103727a8ccb/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L98)| avg = 1229 ms, std_dev = 0 ms\r\n|[ForkJoinCPUBoundPool](https://github.com/jitsi/ice4j/blob/635d8b98d3cb8d5405dce7a434eb8103727a8ccb/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L126)| avg = 1223 ms, std_dev = 0 ms\r\n|[PacketQueueOld](https://github.com/mstyura/ice4j/blob/do-no-block-threads-inside-PacketsQueue-with-PacketQueueOld-benchmark/src/test/java/org/ice4j/util/PacketQueueBenchmarkTests.java#L138)| avg = 1427 ms, std_dev = 1 ms\r\n\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|iterations | 100\r\n|numberOfQueues | **800**\r\n|numberOfItemsInQueue | 1000\r\n|singleQueueItemProcessingWeight | 300\r\n|ThreadPerQueuePool| avg = 925 ms, std_dev = 2 ms\r\n|CachedThreadPerQueuePool| avg = 846 ms, std_dev = 0 ms\r\n|FixedSizeCPUBoundPool| avg = 821 ms, std_dev = 0 ms\r\n|ForkJoinCPUBoundPool| avg = 763 ms, std_dev = 0 ms\r\n|PacketQueueOld| avg = 923 ms, std_dev = 1 ms | # probably faster than `ThreadPerQueuePool` here because all threads created eagerly, instead of lazily by `ExecutorService`.\r\n\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|iterations | 100\r\n|numberOfQueues | **400**\r\n|numberOfItemsInQueue | 1000\r\n|singleQueueItemProcessingWeight | 300\r\n|ThreadPerQueuePool| avg = 446 ms, std_dev = 0 ms\r\n|CachedThreadPerQueuePool| avg = 422 ms, std_dev = 0 ms\r\n|FixedSizeCPUBoundPool| avg = 413 ms, std_dev = 0 ms\r\n|ForkJoinCPUBoundPool| avg = 411 ms, std_dev = 0 ms\r\n|PacketQueueOld| avg = 473 ms, std_dev = 1 ms\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|iterations | 100\r\n|numberOfQueues | **200**\r\n|numberOfItemsInQueue | 1000\r\n|singleQueueItemProcessingWeight | 300\r\n|ThreadPerQueuePool| avg = 238 ms, std_dev = 0 ms\r\n|CachedThreadPerQueuePool| avg = 191 ms, std_dev = 0 ms\r\n|FixedSizeCPUBoundPool| avg = 207 ms, std_dev = 0 ms\r\n|ForkJoinCPUBoundPool| avg = 209 ms, std_dev = 0 ms\r\n|PacketQueueOld| avg = 240 ms, std_dev = 1 ms\r\n\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|iterations | 100\r\n|numberOfQueues | **100**\r\n|numberOfItemsInQueue | 1000\r\n|singleQueueItemProcessingWeight | 300\r\n|ThreadPerQueuePool| avg = 121 ms, std_dev = 0 ms\r\n|CachedThreadPerQueuePool| avg = 113 ms, std_dev = 0 ms\r\n|FixedSizeCPUBoundPool| avg = 98 ms, std_dev = 0 ms\r\n|ForkJoinCPUBoundPool| avg = 106 ms, std_dev = 0 ms\r\n|PacketQueueOld| avg = 122 ms, std_dev = 0 ms\r\n\r\n\r\n|change set|PR #157|\r\n|-|-|\r\n|OS |Ubuntu 18.04, 4.13.0-46-generic\r\n|CPU | i7-4790 CPU\r\n|RAM | 16 Gb\r\n|iterations | 100\r\n|numberOfQueues | **5000**\r\n|numberOfItemsInQueue | **400**\r\n|singleQueueItemProcessingWeight | 300\r\n|ThreadPerQueuePool| avg = 2196 ms, std_dev = 17 ms\r\n|CachedThreadPerQueuePool| avg = 2179 ms, std_dev = 2 ms\r\n|FixedSizeCPUBoundPool| avg = 2057 ms, std_dev = 0 ms\r\n|ForkJoinCPUBoundPool| avg = 1900 ms, std_dev = 0 ms\r\n|PacketQueueOld| avg = 2541 ms, std_dev = 12 ms"},"comment":{"url":"https://api.github.com/repos/jitsi/ice4j/issues/comments/446908067","html_url":"https://github.com/jitsi/ice4j/pull/157#issuecomment-446908067","issue_url":"https://api.github.com/repos/jitsi/ice4j/issues/157","id":446908067,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NjkwODA2Nw==","user":{"login":"mstyura","id":385639,"node_id":"MDQ6VXNlcjM4NTYzOQ==","avatar_url":"https://avatars1.githubusercontent.com/u/385639?v=4","gravatar_id":"","url":"https://api.github.com/users/mstyura","html_url":"https://github.com/mstyura","followers_url":"https://api.github.com/users/mstyura/followers","following_url":"https://api.github.com/users/mstyura/following{/other_user}","gists_url":"https://api.github.com/users/mstyura/gists{/gist_id}","starred_url":"https://api.github.com/users/mstyura/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mstyura/subscriptions","organizations_url":"https://api.github.com/users/mstyura/orgs","repos_url":"https://api.github.com/users/mstyura/repos","events_url":"https://api.github.com/users/mstyura/events{/privacy}","received_events_url":"https://api.github.com/users/mstyura/received_events","type":"User","site_admin":false},"created_at":"2018-12-13T09:53:59Z","updated_at":"2018-12-13T09:53:59Z","author_association":"CONTRIBUTOR","body":"Hi, @damencho,\r\nThe log suggest that the compilation issue caused by #155, because I've removed this method."}}
{"actor":{"display_login":"mstyura"}}