Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use FFD algorithm for ArrayUtil.joinWithMax #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pocke
Copy link
Contributor

@pocke pocke commented Oct 25, 2021

It reduces the number of queries, especially the user has many watching repositories.

Problem

In my environment, the watching queries are too many because I watch 1000+ repositories.
It introduces a serious performance issue. Because Jasper executes one query per one iteration. So if it has 100 queries, Jasper needs 100 iterations to execute all 100 queries.

Solution

Improve the algorithm to pack queries.
I replace the joinWithMax implementation with FFD (First Fit Decreasing) algorithm.
The algorithm is super simple.

  1. Sort values by decreasing order by length.
    • It means sorting ['short', 'looooong'] to ['looooong', 'short']
  2. Pack the values to result with the following rules.
    • If the first result has the capacity, push the value to the result.
      Else, try the next result.
    • If all result has no capacity for the value, allocate new result.

This algorithm is an approximation algorithm, but it is better than the current naive implementation, and it is enough I think.
See this article for more details. https://en.wikipedia.org/wiki/Bin_packing_problem

Benchmark

I benchmarked my watching query. By this change, the watching queries count is reduced from 121 to 114. It is still super large but better a little.

I benchmarked with the following code:

diff --git a/src/Renderer/Repository/Polling/StreamClient/SystemStreamWatchingClient.ts b/src/Renderer/Repository/Polling/StreamClient/SystemStreamWatchingClient.ts
index 73a7706..ad24522 100644
--- a/src/Renderer/Repository/Polling/StreamClient/SystemStreamWatchingClient.ts
+++ b/src/Renderer/Repository/Polling/StreamClient/SystemStreamWatchingClient.ts
@@ -18,8 +18,8 @@ export class SystemStreamWatchingClient extends StreamClient {
     // note: query max length is 256
     // https://docs.github.com/en/free-pro-team@latest/github/searching-for-information-on-github/troubleshooting-search-queries#limitations-on-query-length
     const updatedLength = ` updated:>=YYYY-MM-DDThh:mm:ssZ`.length;
     const queries = ArrayUtil.joinWithMax(watchings.map(w => `repo:${w}`), 256 - updatedLength);
+    console.log(`queries.length: ${queries.length}`)
     return queries;
   }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant