Skip to content

Commit 0f9d7e7

Browse files
authored
repo sync: Use ChangesAreMerged to batch check submitted branches (#400)
Make use of the ChangesAreMerged to make a single network request for all submitted branches, rather than one per branch. We still need to make one request per tracked branch for now.
1 parent e2f3b24 commit 0f9d7e7

File tree

2 files changed

+51
-49
lines changed

2 files changed

+51
-49
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
kind: Changed
2+
body: 'repo sync: Reduce the number of network requests made to check status of submitted branches.'
3+
time: 2024-09-13T18:50:02.036293-07:00

repo_sync.go

Lines changed: 48 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -243,49 +243,64 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
243243
// 2. Branches that the user submitted PRs for manually
244244
// with 'gh pr create' or similar.
245245
//
246-
// For the first, we can perform a cheaper API call to check the CR status.
246+
// For the first, we can perform a cheap API call to check the CR status.
247247
// For the second, we need to find recently merged PRs with that branch
248248
// name, and match the remote head SHA to the branch head SHA.
249249
//
250250
// We'll try to do these checks concurrently.
251251

252-
submittedch := make(chan *submittedBranch)
253-
trachedch := make(chan *trackedBranch)
252+
var (
253+
submittedBranches []*submittedBranch
254+
trackedBranches []*trackedBranch
255+
)
256+
for _, b := range knownBranches {
257+
if b.Change != nil {
258+
b := &submittedBranch{
259+
Name: b.Name,
260+
Change: b.Change.ChangeID(),
261+
}
262+
submittedBranches = append(submittedBranches, b)
263+
} else {
264+
// TODO:
265+
// Filter down to only branches that have
266+
// a remote tracking branch:
267+
// either $remote/$UpstreamBranch or $remote/$branch exists.
268+
b := &trackedBranch{Name: b.Name}
269+
trackedBranches = append(trackedBranches, b)
270+
}
271+
}
254272

255273
var wg sync.WaitGroup
256-
for range min(runtime.GOMAXPROCS(0), len(knownBranches)) {
274+
if len(submittedBranches) > 0 {
257275
wg.Add(1)
258276
go func() {
259277
defer wg.Done()
260278

261-
// We'll nil these out once they're closed.
262-
submittedch := submittedch
263-
trachedch := trachedch
264-
265-
for submittedch != nil || trachedch != nil {
266-
select {
267-
case b, ok := <-submittedch:
268-
if !ok {
269-
submittedch = nil
270-
continue
271-
}
279+
changeIDs := make([]forge.ChangeID, len(submittedBranches))
280+
for i, b := range submittedBranches {
281+
changeIDs[i] = b.Change
282+
}
272283

273-
// TODO: Once we're recording GraphQL IDs in the store,
274-
// we can combine all submitted PRs into one query.
275-
merged, err := remoteRepo.ChangesAreMerged(ctx, []forge.ChangeID{b.Change})
276-
if err != nil {
277-
log.Error("Failed to query CR status", "change", b.Change, "error", err)
278-
continue
279-
}
284+
merged, err := remoteRepo.ChangesAreMerged(ctx, changeIDs)
285+
if err != nil {
286+
log.Error("Failed to query CR status", "error", err)
287+
return
288+
}
280289

281-
b.Merged = merged[0]
290+
for i, m := range merged {
291+
submittedBranches[i].Merged = m
292+
}
293+
}()
294+
}
282295

283-
case b, ok := <-trachedch:
284-
if !ok {
285-
trachedch = nil
286-
continue
287-
}
296+
if len(trackedBranches) > 0 {
297+
trackedch := make(chan *trackedBranch)
298+
for range min(runtime.GOMAXPROCS(0), len(trackedBranches)) {
299+
wg.Add(1)
300+
go func() {
301+
defer wg.Done()
288302

303+
for b := range trackedch {
289304
changes, err := remoteRepo.FindChangesByBranch(ctx, b.Name, forge.FindChangesOptions{
290305
Limit: 3,
291306
State: forge.ChangeMerged,
@@ -313,30 +328,14 @@ func (cmd *repoSyncCmd) deleteMergedBranches(
313328
}
314329

315330
}
316-
}
317-
}()
318-
}
331+
}()
332+
}
319333

320-
var (
321-
submittedBranches []*submittedBranch
322-
trackedBranches []*trackedBranch
323-
)
324-
for _, b := range knownBranches {
325-
if b.Change != nil {
326-
b := &submittedBranch{
327-
Name: b.Name,
328-
Change: b.Change.ChangeID(),
329-
}
330-
submittedBranches = append(submittedBranches, b)
331-
submittedch <- b
332-
} else {
333-
b := &trackedBranch{Name: b.Name}
334-
trackedBranches = append(trackedBranches, b)
335-
trachedch <- b
334+
for _, b := range trackedBranches {
335+
trackedch <- b
336336
}
337+
close(trackedch)
337338
}
338-
close(submittedch)
339-
close(trachedch)
340339
wg.Wait()
341340

342341
var branchesToDelete []string

0 commit comments

Comments
 (0)