Optimize build-task handler by using object loading view

5 files changed, 45 insertions, 35 deletions

diff --git a/clean/clean.cxx b/clean/clean.cxx
index cebf199..59084d3 100644
--- a/clean/clean.cxx
+++ b/clean/clean.cxx
@@ -299,19 +299,19 @@ namespace brep
     // be made once per tenant package name due to the builds query sorting
     // criteria (see above).
     //
-    using pkg_query = query<buildable_package>;
-    using prep_pkg_query = prepared_query<buildable_package>;
+    using pkg_query = query<build_package_version>;
+    using prep_pkg_query = prepared_query<build_package_version>;
 
     string tnt;
     package_name pkg_name;
     set<version> package_versions;
 
-    pkg_query pq (
-      pkg_query::build_package::id.tenant == pkg_query::_ref (tnt) &&
-      pkg_query::build_package::id.name == pkg_query::_ref (pkg_name));
+    pkg_query pq (pkg_query::buildable                          &&
+                  pkg_query::id.tenant == pkg_query::_ref (tnt) &&
+                  pkg_query::id.name == pkg_query::_ref (pkg_name));
 
     prep_pkg_query pkg_prep_query (
-      conn->prepare_query<buildable_package> ("package-query", pq));
+      conn->prepare_query<build_package_version> ("package-query", pq));
 
     for (bool ne (true); ne; )
     {
@@ -331,11 +331,16 @@ namespace brep
                         ? i->second
                         : default_timeout);
 
-          // @@ Note that this approach doesn't consider the case when both
-          //    the configuration and the package still exists but the package
-          //    now excludes the configuration (configuration is now of the
-          //    legacy class instead of the default class, etc). We should
-          //    probably re-implement it in a way brep-monitor does it.
+          // Note that we don't consider the case when both the configuration
+          // and the package still exist but the package now excludes the
+          // configuration (configuration is now of the legacy class instead
+          // of the default class, etc). Should we handle this case and
+          // re-implement in a way brep-monitor does it? Probably not since
+          // the described situation is not very common and storing some extra
+          // builds which sooner or later will be wiped out due to the timeout
+          // is harmless. The current implementation, however, is simpler and
+          // consumes less resources in runtime (doesn't load build package
+          // objects, etc).
           //
           bool cleanup (
             // Check that the build is not stale.
diff --git a/libbrep/build-package.hxx b/libbrep/build-package.hxx
index 2e3afe0..50294a3 100644
--- a/libbrep/build-package.hxx
+++ b/libbrep/build-package.hxx
@@ -229,6 +229,17 @@ namespace brep
     build_package () = default;
   };
 
+  #pragma db view object(build_package)
+  struct build_package_version
+  {
+    package_id id;
+    upstream_version version;
+
+    // Database mapping.
+    //
+    #pragma db member(version) set(this.version.init (this.id.version, (?)))
+  };
+
   // Packages that can potentially be built.
   //
   // Note that ADL can't find the equal operator, so we use the function call
@@ -243,18 +254,13 @@ namespace brep
     object(build_tenant: build_package::id.tenant == build_tenant::id)
   struct buildable_package
   {
-    package_id id;
-    upstream_version version;
+    shared_ptr<build_package> package;
 
     bool archived; // True if the tenant the package belongs to is archived.
 
     // Present if the tenant the package belongs to is interactive.
     //
     optional<string> interactive;
-
-    // Database mapping.
-    //
-    #pragma db member(version) set(this.version.init (this.id.version, (?)))
   };
 
   #pragma db view                                                      \
diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx
index a2e1f56..b1f7658 100644
--- a/mod/mod-build-task.cxx
+++ b/mod/mod-build-task.cxx
@@ -1027,7 +1027,9 @@ handle (request& rq, response& rs)
         //
         for (auto& bp: packages)
         {
-          id = move (bp.id);
+          shared_ptr<build_package>& p (bp.package);
+
+          id = p->id;
 
           // If we are in the random package ordering mode, then cache the
           // tenant the start offset refers to, if not cached yet, and check
@@ -1130,8 +1132,7 @@ handle (request& rq, response& rs)
             pkg_query pq (pkg_query::build_tenant::id == id.tenant);
             for (auto& tp: build_db_->query<buildable_package> (pq))
             {
-              shared_ptr<build_package> p (
-                build_db_->load<build_package> (tp.id));
+              shared_ptr<build_package>& p (tp.package);
 
               build_db_->load (*p, p->constraints_section);
 
@@ -1176,7 +1177,7 @@ handle (request& rq, response& rs)
                   b = make_shared<build> (
                     move (bid.package.tenant),
                     move (bid.package.name),
-                    bp.version,
+                    p->version,
                     move (bid.target),
                     move (bid.target_config_name),
                     move (bid.package_config_name),
@@ -1219,8 +1220,6 @@ handle (request& rq, response& rs)
             }
           }
 
-          shared_ptr<build_package> p (build_db_->load<build_package> (id));
-
           for (build_package_config& pc: p->configs)
           {
             pkg_config_name = pc.name;
@@ -1301,7 +1300,7 @@ handle (request& rq, response& rs)
                 {
                   b = make_shared<build> (move (bid.package.tenant),
                                           move (bid.package.name),
-                                          move (bp.version),
+                                          p->version,
                                           move (bid.target),
                                           move (bid.target_config_name),
                                           move (bid.package_config_name),
diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx
index 0c9e57a..bd87d84 100644
--- a/mod/mod-builds.cxx
+++ b/mod/mod-builds.cxx
@@ -889,9 +889,9 @@ handle (request& rq, response& rs)
 
         for (auto& bp: build_db_->query<buildable_package> (q))
         {
-          id = move (bp.id);
+          shared_ptr<build_package>& p (bp.package);
 
-          shared_ptr<build_package> p (build_db_->load<build_package> (id));
+          id = p->id;
 
           // Note: load the constrains section lazily.
           //
@@ -1027,11 +1027,11 @@ handle (request& rq, response& rs)
         // Iterate over packages and print unbuilt configurations. Skip the
         // appropriate number of them first (for page number greater than one).
         //
-        for (auto& p: packages)
+        for (auto& bp: packages)
         {
-          id = move (p.id);
+          shared_ptr<build_package>& p (bp.package);
 
-          shared_ptr<build_package> bp (build_db_->load<build_package> (id));
+          id = p->id;
 
           // Copy configuration/toolchain combinations for this package,
           // skipping excluded configurations.
@@ -1040,7 +1040,7 @@ handle (request& rq, response& rs)
 
           // Load the constrains section lazily.
           //
-          for (const build_package_config& pc: bp->configs)
+          for (const build_package_config& pc: p->configs)
           {
             // Filter by package config name.
             //
@@ -1054,10 +1054,10 @@ handle (request& rq, response& rs)
 
                 assert (i != target_conf_map_->end ());
 
-                if (!bp->constraints_section.loaded ())
-                  build_db_->load (*bp, bp->constraints_section);
+                if (!p->constraints_section.loaded ())
+                  build_db_->load (*p, p->constraints_section);
 
-                if (!exclude (pc, bp->builds, bp->constraints, *i->second))
+                if (!exclude (pc, p->builds, p->constraints, *i->second))
                   unbuilt_configs.insert (
                     config_toolchain {ct.target,
                                       ct.target_config,
@@ -1095,7 +1095,7 @@ handle (request& rq, response& rs)
             s << TABLE(CLASS="proplist build")
               <<   TBODY
               <<     TR_NAME (id.name, string (), root, id.tenant)
-              <<     TR_VERSION (id.name, p.version, root, id.tenant)
+              <<     TR_VERSION (id.name, p->version, root, id.tenant)
               <<     TR_VALUE ("toolchain",
                                string (ct.toolchain_name) + '-' +
                                ct.toolchain_version.string ())
diff --git a/monitor/monitor.cxx b/monitor/monitor.cxx
index eba6e2b..05ebf9d 100644
--- a/monitor/monitor.cxx
+++ b/monitor/monitor.cxx
@@ -821,7 +821,7 @@ namespace brep
 
           for (auto& bp: bps)
           {
-            shared_ptr<build_package> p (db.load<build_package> (bp.id));
+            shared_ptr<build_package>& p (bp.package);
 
             db.load (*p, p->constraints_section);