Integrated: 1529: Optimize Census namespace instantiation
Erik Joelsson
erikj at openjdk.org
Thu Aug 11 17:47:04 UTC 2022
On Fri, 5 Aug 2022 17:52:55 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
> This patch adds a new factory method `Census::parseNamespace`, which enables initialization of a single Namespace directly from a HostedRepository (without having to clone the census repo to local disk first).
>
> The motivation behind this is bot performance. The Census is loaded in many WorkItems. Cloning/fetching the complete census repo to local disk is relatively expensive compared to a couple of REST calls to just read files directly from the remote repo. Especially as our REST client already leverages caching through ETag. This particular optimization can however only be applied to the IssueNotifier in the NotifyBot, where I doubt it will make that much of a difference. The main performance gain is realized in a different bot which currently isn't public (but these gains will impact all Skara users).
>
> I chose to at least for now, only apply this for initializing a single Namespace as that can be done by reading only 2 files from the remote, so the performance gain is very clear. Other bots either need to read considerably more files, or have the need to update the census. This optimization idea may be expanded in the future if I see a need and noticeable gains from it.
>
> In order to make HostedRepository available to the Census class, I had to get rid of several existing module dependencies to avoid circular dependencies. Fortunately this wasn't too hard. It made no sense to me that `org.openjdk.skara.forge` would require `org.openjdk.skara.census`. There was just one static method `PullRequestUtils` in the forge module that used a reference to the census module, and this static method was only used from one module `org.openjdk.skara.bots.pr`, so I simply moved it to `CheckablePullRequest` which is kind of a utility class in that module. The rest of the offending dependencies were just unused, so I cleaned out a fair few of those.
>
> In the `IssueNotifier`, in addition to using the new initialization method, I also added lazy initialization and reusing of the `CensusInstance` class, to avoid re-initializing the same data for every round in loops.
This pull request has now been integrated.
Changeset: f9d5585f
Author: Erik Joelsson <erikj at openjdk.org>
URL: https://git.openjdk.org/skara/commit/f9d5585f97354abe575839d7f7d1bab0d05a4d06
Stats: 209 lines in 18 files changed: 120 ins; 64 del; 25 mod
1529: Optimize Census namespace instantiation
Reviewed-by: ihse
-------------
PR: https://git.openjdk.org/skara/pull/1348
More information about the skara-dev
mailing list