From 6ece001dccf59aa60e86a9fabfcfb9063f0bf799 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett@linaro.org>
Date: Mon, 16 Sep 2024 12:15:56 +0100
Subject: [PATCH] Collapse clean and non clean build requests on clean builders

This is to address https://github.com/llvm/llvm-zorg/issues/250.
Where build requests were piling up on slower builders because
they were clean/don't clean/clean/ don't clean etc. and we could
not merge those requests because of that difference.

Galina suggested that if we knew that the builder was going to
clean the build first anyway, we could merge them. This commit
implements that.

We do this by first setting a "clean" attribute on the builder's
factory object, that we can find later via in the collapse callback.

So far only ClangBuilder passes this on but any builder can opt
in by copying what it does.

In the collapse function when comparing properties we delete the
"clean_obj" key from both sets of properties if we know that the
builder's factory is always clean.

With some logging (not included in this commit) we can see it now
collapses a clean/non-clean pair of requests:
```
collapseRequests
selfBuildSetProperties: {'scheduler': ('main:clang,clang-tools-extra,compiler-rt,flang,lld,llvm,mlir', 'Scheduler')}
otherBuildSetProperties: {'scheduler': ('main:clang,clang-tools-extra,compiler-rt,flang,lld,llvm,mlir', 'Scheduler'), 'clean_obj': (True, 'Change')}
Removing clean_obj from build set properties.
Build set properties were the same.
```

This has beeen tested by my colleague Omair Javaid on a test build
master. Of course we only tested it with one of Linaro's builders,
so it's possible we see other side effects in production.
---
 zorg/buildbot/builders/ClangBuilder.py |  3 ++-
 zorg/buildbot/process/buildrequest.py  | 22 ++++++++++++++--------
 zorg/buildbot/process/factory.py       |  6 +++++-
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/zorg/buildbot/builders/ClangBuilder.py b/zorg/buildbot/builders/ClangBuilder.py
index a003f3987..fffc14618 100644
--- a/zorg/buildbot/builders/ClangBuilder.py
+++ b/zorg/buildbot/builders/ClangBuilder.py
@@ -278,7 +278,8 @@ def _getClangCMakeBuildFactory(
     f = LLVMBuildFactory(
             depends_on_projects=depends_on_projects,
             llvm_srcdir='llvm',
-            enable_runtimes=enable_runtimes)
+            enable_runtimes=enable_runtimes,
+            clean=clean)
 
     # Checkout the latest code for LNT
     # and the test-suite separately. Le's do this first,
diff --git a/zorg/buildbot/process/buildrequest.py b/zorg/buildbot/process/buildrequest.py
index 594a09d25..1f5043d11 100644
--- a/zorg/buildbot/process/buildrequest.py
+++ b/zorg/buildbot/process/buildrequest.py
@@ -22,14 +22,20 @@ def collapseRequests(master, builder, req1, req2):
         ('buildsets', str(req2['buildsetid'])))
 
     # Fetch the buildset properties.
-    selfBuildsetPoperties = yield \
-        master.db.buildsets.getBuildsetProperties(
-            str(req1['buildsetid'])
-        )
-    otherBuildsetPoperties = yield \
-        master.db.buildsets.getBuildsetProperties(
-            str(req2['buildsetid'])
-        )
+    selfBuildsetPoperties = yield master.db.buildsets.getBuildsetProperties(
+        str(req1["buildsetid"])
+    )
+    otherBuildsetPoperties = yield master.db.buildsets.getBuildsetProperties(
+        str(req2["buildsetid"])
+    )
+
+    # If the build is going to be a clean build anyway, we can collapse a clean
+    # build and a non-clean build.
+    if getattr(builder.config.factory, "clean"):
+        if 'clean_obj' in selfBuildsetPoperties:
+            del selfBuildsetPoperties["clean_obj"]
+        if 'clean_obj' in otherBuildsetPoperties:
+            del otherBuildsetPoperties["clean_obj"]
 
     # Check buildsets properties and do not collapse
     # if properties do not match. This includes the check
diff --git a/zorg/buildbot/process/factory.py b/zorg/buildbot/process/factory.py
index b15bbaa19..e8af8faa0 100644
--- a/zorg/buildbot/process/factory.py
+++ b/zorg/buildbot/process/factory.py
@@ -35,11 +35,15 @@ class LLVMBuildFactory(BuildFactory):
             & etc.
     """
 
-    def __init__(self, steps=None, depends_on_projects=None, hint=None, **kwargs):
+    def __init__(
+        self, steps=None, depends_on_projects=None, hint=None, clean=False, **kwargs
+    ):
         # Cannot use "super" here as BuildFactory is an old style class.
         BuildFactory.__init__(self, steps)
 
         self.hint = hint
+
+        self.clean = clean
         
         # Handle the dependencies.
         if depends_on_projects is None: