Clean up the autoconf caching

This makes the stapconf caching process quite a bit more transparent.
The options are now cached in a header file as #defines, and this header
file is a normal build dependency instead of calling so many make
$(shell ...) commands.

  * buildrun.cxx (compile_pass): Pull in autoconf options in a header
  of #defines rather than -DXXX, and make that header a build dependency.
  * buildrun.cxx (output_autoconf): New function to consolidate the
  computation of each autoconf test.
  * cache.cxx (add_to_cache, get_from_cache, clean_cache): Start treating
  the stapconf header as a first-class cached item.
  * cache.h: Move definitions of things only needed in cache.cxx
  * hash.cxx (find_stapconf_hash): Generate stapconf_name as a .h now.
  * main.cxx (main): Default the stapconf_name based on getpid().

7 files changed, 176 insertions, 97 deletions

diff --git a/ChangeLog b/ChangeLog
index 9a3c3060..d8152ed3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-02-07  Josh Stone  <jistone@redhat.com>
+
+	* buildrun.cxx (compile_pass): Pull in autoconf options in a header
+	of #defines rather than -DXXX, and make that header a build dependency.
+	* buildrun.cxx (output_autoconf): New function to consolidate the
+	computation of each autoconf test.
+	* cache.cxx (add_to_cache, get_from_cache, clean_cache): Start treating
+	the stapconf header as a first-class cached item.
+	* cache.h: Move definitions of things only needed in cache.cxx
+	* hash.cxx (find_stapconf_hash): Generate stapconf_name as a .h now.
+	* main.cxx (main): Default the stapconf_name based on getpid().
+
 2009-02-06  Josh Stone  <jistone@redhat.com>
 
 	* hash.cxx (find_hash): Compute a separate script-independent hash
diff --git a/buildrun.cxx b/buildrun.cxx
index 5b4db922..3a6e20ae 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -60,6 +60,24 @@ run_make_cmd(systemtap_session& s, string& make_cmd)
   return rc;
 }
 
+static void
+output_autoconf(systemtap_session& s, ofstream& o, const char *autoconf_c,
+                const char *deftrue, const char *deffalse)
+{
+  o << "\t";
+  if (s.verbose < 4)
+    o << "@";
+  o << "if $(CHECK_BUILD) $(SYSTEMTAP_RUNTIME)/" << autoconf_c;
+  if (s.verbose < 5)
+    o << " > /dev/null 2>&1";
+  o << "; then ";
+  if (deftrue)
+    o << "echo \"#define " << deftrue << " 1\"";
+  if (deffalse)
+    o << "; else echo \"#define " << deffalse << " 1\"";
+  o << "; fi >> $@" << endl;
+}
+
 int
 compile_pass (systemtap_session& s)
 {
@@ -86,7 +104,8 @@ compile_pass (systemtap_session& s)
   o << "_KBUILD_CFLAGS := $(call flags,KBUILD_CFLAGS)" << endl;
 
   o << "stap_check_gcc = $(shell " << superverbose << " if $(CC) $(1) -S -o /dev/null -xc /dev/null > /dev/null 2>&1; then echo \"$(1)\"; else echo \"$(2)\"; fi)" << endl;
-  o << "stap_check_build = $(shell " << superverbose << " if $(CC) $(KBUILD_CPPFLAGS) $(CPPFLAGS) $(LINUXINCLUDE) $(_KBUILD_CFLAGS) $(CFLAGS_KERNEL) $(EXTRA_CFLAGS) $(CFLAGS) -DKBUILD_BASENAME=\\\"" << s.module_name << "\\\" -Werror -S -o /dev/null -xc $(1) " << redirecterrors << " ; then echo \"$(2)\"; else echo \"$(3)\"; fi)" << endl;
+  o << "CHECK_BUILD := $(CC) $(KBUILD_CPPFLAGS) $(CPPFLAGS) $(LINUXINCLUDE) $(_KBUILD_CFLAGS) $(CFLAGS_KERNEL) $(EXTRA_CFLAGS) $(CFLAGS) -DKBUILD_BASENAME=\\\"" << s.module_name << "\\\" -Werror -S -o /dev/null -xc " << endl;
+  o << "stap_check_build = $(shell " << superverbose << " if $(CHECK_BUILD) $(1) " << redirecterrors << " ; then echo \"$(2)\"; else echo \"$(3)\"; fi)" << endl;
 
   o << "SYSTEMTAP_RUNTIME = \"" << s.runtime_path << "\"" << endl;
 
@@ -109,45 +128,36 @@ compile_pass (systemtap_session& s)
   string stapconf_cflags = "STAPCONF_CFLAGS";
   o << stapconf_cflags << " :=" << endl;
 
-  if (s.use_cache)
-    {
-      o << "STAPCONF_CACHE_FILE = " << s.stapconf_path << endl;
-      o << stapconf_cflags << " += $(shell cat $(STAPCONF_CACHE_FILE) 2>/dev/null)" << endl;
-      o << "ifeq (,$(strip $(" << stapconf_cflags << ")))" << endl;
-    }
-
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-hrtimer-rel.c, -DSTAPCONF_HRTIMER_REL,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-hrtimer-getset-expires.c, -DSTAPCONF_HRTIMER_GETSET_EXPIRES,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-inode-private.c, -DSTAPCONF_INODE_PRIVATE,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-constant-tsc.c, -DSTAPCONF_CONSTANT_TSC,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-tsc-khz.c, -DSTAPCONF_TSC_KHZ,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-ktime-get-real.c, -DSTAPCONF_KTIME_GET_REAL,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-x86-uniregs.c, -DSTAPCONF_X86_UNIREGS,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-nameidata.c, -DSTAPCONF_NAMEIDATA_CLEANUP,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-unregister-kprobes.c, -DSTAPCONF_UNREGISTER_KPROBES,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-real-parent.c, -DSTAPCONF_REAL_PARENT,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-uaccess.c, -DSTAPCONF_LINUX_UACCESS_H,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-oneachcpu-retry.c, -DSTAPCONF_ONEACHCPU_RETRY,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-dpath-path.c, -DSTAPCONF_DPATH_PATH,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-synchronize-sched.c, -DSTAPCONF_SYNCHRONIZE_SCHED,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-task-uid.c, -DSTAPCONF_TASK_UID,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-vm-area.c, -DSTAPCONF_VM_AREA,)" << endl;
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-procfs-owner.c, -DSTAPCONF_PROCFS_OWNER,)" << endl;
+  o << "STAPCONF_HEADER := " << s.tmpdir << "/" << s.stapconf_name << endl;
+  o << s.translated_source << ": $(STAPCONF_HEADER)" << endl;
+  o << "$(STAPCONF_HEADER):" << endl;
+  o << "\t@echo -n > $@" << endl;
+  output_autoconf(s, o, "autoconf-hrtimer-rel.c", "STAPCONF_HRTIMER_REL", NULL);
+  output_autoconf(s, o, "autoconf-hrtimer-getset-expires.c", "STAPCONF_HRTIMER_GETSET_EXPIRES", NULL);
+  output_autoconf(s, o, "autoconf-inode-private.c", "STAPCONF_INODE_PRIVATE", NULL);
+  output_autoconf(s, o, "autoconf-constant-tsc.c", "STAPCONF_CONSTANT_TSC", NULL);
+  output_autoconf(s, o, "autoconf-tsc-khz.c", "STAPCONF_TSC_KHZ", NULL);
+  output_autoconf(s, o, "autoconf-ktime-get-real.c", "STAPCONF_KTIME_GET_REAL", NULL);
+  output_autoconf(s, o, "autoconf-x86-uniregs.c", "STAPCONF_X86_UNIREGS", NULL);
+  output_autoconf(s, o, "autoconf-nameidata.c", "STAPCONF_NAMEIDATA_CLEANUP", NULL);
+  output_autoconf(s, o, "autoconf-unregister-kprobes.c", "STAPCONF_UNREGISTER_KPROBES", NULL);
+  output_autoconf(s, o, "autoconf-real-parent.c", "STAPCONF_REAL_PARENT", NULL);
+  output_autoconf(s, o, "autoconf-uaccess.c", "STAPCONF_LINUX_UACCESS_H", NULL);
+  output_autoconf(s, o, "autoconf-oneachcpu-retry.c", "STAPCONF_ONEACHCPU_RETRY", NULL);
+  output_autoconf(s, o, "autoconf-dpath-path.c", "STAPCONF_DPATH_PATH", NULL);
+  output_autoconf(s, o, "autoconf-synchronize-sched.c", "STAPCONF_SYNCHRONIZE_SCHED", NULL);
+  output_autoconf(s, o, "autoconf-task-uid.c", "STAPCONF_TASK_UID", NULL);
+  output_autoconf(s, o, "autoconf-vm-area.c", "STAPCONF_VM_AREA", NULL);
+  output_autoconf(s, o, "autoconf-procfs-owner.c", "STAPCONF_PROCFS_OWNER", NULL);
 
 #if 0
   /* NB: For now, the performance hit of probe_kernel_read/write (vs. our
    * homegrown safe-access functions) is deemed undesireable, so we'll skip
    * this autoconf. */
-  o << stapconf_cflags << " += $(call stap_check_build, $(SYSTEMTAP_RUNTIME)/autoconf-probe-kernel.c, -DSTAPCONF_PROBE_KERNEL,)" << endl;
+  output_autoconf(s, o, "autoconf-probe-kernel.c", "STAPCONF_PROBE_KERNEL", NULL);
 #endif
 
-  if (s.use_cache)
-    {
-      o << "$(shell echo $(" << stapconf_cflags << ") > $(STAPCONF_CACHE_FILE))" << endl;
-      o << "endif" << endl;
-    }
-
-  o << module_cflags << " += $(" << stapconf_cflags << ")" << endl;
+  o << module_cflags << " += -include $(STAPCONF_HEADER)" << endl;
 
   for (unsigned i=0; i<s.macros.size(); i++)
     o << "EXTRA_CFLAGS += -D " << lex_cast_qstring(s.macros[i]) << endl;
diff --git a/cache.cxx b/cache.cxx
index 35a37a00..86f7213a 100644
--- a/cache.cxx
+++ b/cache.cxx
@@ -25,9 +25,38 @@ extern "C" {
 using namespace std;
 
 
+#define SYSTEMTAP_CACHE_MAX_FILENAME "cache_mb_limit"
+#define SYSTEMTAP_CACHE_DEFAULT_MB 64
+
+struct cache_ent_info {
+  string path;
+  bool is_module;
+  size_t size;
+  long weight;  //lower == removed earlier
+
+  cache_ent_info(const string& path, bool is_module);
+  bool operator<(const struct cache_ent_info& other) const
+  { return weight < other.weight; }
+  void unlink() const;
+};
+
+static void clean_cache(systemtap_session& s);
+
+
 void
 add_to_cache(systemtap_session& s)
 {
+  string stapconf_src_path = s.tmpdir + "/" + s.stapconf_name;
+  if (s.verbose > 1)
+    clog << "Copying " << stapconf_src_path << " to " << s.stapconf_path << endl;
+  if (copy_file(stapconf_src_path.c_str(), s.stapconf_path.c_str()) != 0)
+    {
+      cerr << "Copy failed (\"" << stapconf_src_path << "\" to \""
+	   << s.stapconf_path << "\"): " << strerror(errno) << endl;
+      s.use_cache = false;
+      return;
+    }
+
   string module_src_path = s.tmpdir + "/" + s.module_name + ".ko";
   if (s.verbose > 1)
     clog << "Copying " << module_src_path << " to " << s.hash_path << endl;
@@ -65,14 +94,38 @@ add_to_cache(systemtap_session& s)
 bool
 get_from_cache(systemtap_session& s)
 {
+  string stapconf_dest_path = s.tmpdir + "/" + s.stapconf_name;
   string module_dest_path = s.tmpdir + "/" + s.module_name + ".ko";
   string c_src_path = s.hash_path;
-  int fd_module, fd_c;
+  int fd_stapconf, fd_module, fd_c;
 
   if (c_src_path.rfind(".ko") == (c_src_path.size() - 3))
     c_src_path.resize(c_src_path.size() - 3);
   c_src_path += ".c";
 
+  // See if stapconf exists
+  fd_stapconf = open(s.stapconf_path.c_str(), O_RDONLY);
+  if (fd_stapconf == -1)
+    {
+      // It isn't in cache.
+      return false;
+    }
+
+  // Copy the stapconf header file to the destination
+  if (copy_file(s.stapconf_path.c_str(), stapconf_dest_path.c_str()) != 0)
+    {
+      cerr << "Copy failed (\"" << s.stapconf_path << "\" to \""
+	   << stapconf_dest_path << "\"): " << strerror(errno) << endl;
+      close(fd_stapconf);
+      return false;
+    }
+
+  // We're done with this file handle.
+  close(fd_stapconf);
+
+  if (s.verbose > 1)
+    clog << "Pass 3: using cached " << s.stapconf_path << endl;
+
   // See if module exists
   fd_module = open(s.hash_path.c_str(), O_RDONLY);
   if (fd_module == -1)
@@ -146,7 +199,7 @@ get_from_cache(systemtap_session& s)
 }
 
 
-void
+static void
 clean_cache(systemtap_session& s)
 {
   if (s.cache_path != "")
@@ -170,7 +223,9 @@ clean_cache(systemtap_session& s)
     	  cache_mb_max = SYSTEMTAP_CACHE_DEFAULT_MB;
 
           if (s.verbose > 1)
-            clog << "Cache limit file " << s.cache_path << "/" << SYSTEMTAP_CACHE_MAX_FILENAME << " missing, creating default." << endl;
+            clog << "Cache limit file " << s.cache_path << "/"
+              << SYSTEMTAP_CACHE_MAX_FILENAME
+              << " missing, creating default." << endl;
         }
 
       //glob for all kernel modules in the cache dir
@@ -179,33 +234,44 @@ clean_cache(systemtap_session& s)
       glob(glob_str.c_str(), 0, NULL, &cache_glob);
 
 
-      set<struct cache_ent_info, struct weight_sorter> cache_contents;
+      set<struct cache_ent_info> cache_contents;
       unsigned long cache_size_b = 0;
 
       //grab info for each cache entry (.ko and .c)
       for (unsigned int i = 0; i < cache_glob.gl_pathc; i++)
         {
-          struct cache_ent_info cur_info;
           string cache_ent_path = cache_glob.gl_pathv[i];
-          long cur_size = 0;
+          cache_ent_path.resize(cache_ent_path.length() - 3);
+
+          struct cache_ent_info cur_info(cache_ent_path, true);
+          if (cur_info.size != 0 && cur_info.weight != 0)
+            {
+              cache_size_b += cur_info.size;
+              cache_contents.insert(cur_info);
+            }
+        }
 
-          cache_ent_path = cache_ent_path.substr(0, cache_ent_path.length() - 3);
-          cur_info.path = cache_ent_path;
-          cur_info.weight = get_cache_file_weight(cache_ent_path);
+      globfree(&cache_glob);
 
-          cur_size = get_cache_file_size(cache_ent_path);
-          cur_info.size = cur_size;
-          cache_size_b += cur_size;
+      //grab info for each stapconf cache entry (.h)
+      glob_str = s.cache_path + "/*/*.h";
+      glob(glob_str.c_str(), 0, NULL, &cache_glob);
+      for (unsigned int i = 0; i < cache_glob.gl_pathc; i++)
+        {
+          string cache_ent_path = cache_glob.gl_pathv[i];
+          cache_ent_path.resize(cache_ent_path.length() - 3);
 
+          struct cache_ent_info cur_info(cache_ent_path, false);
           if (cur_info.size != 0 && cur_info.weight != 0)
             {
+              cache_size_b += cur_info.size;
               cache_contents.insert(cur_info);
             }
         }
 
       globfree(&cache_glob);
 
-      set<struct cache_ent_info, struct weight_sorter>::iterator i;
+      set<struct cache_ent_info>::iterator i;
       unsigned long r_cache_size = cache_size_b;
       string removed_dirs = "";
 
@@ -216,8 +282,8 @@ clean_cache(systemtap_session& s)
             break;
 
           //remove this (*i) cache_entry, add to removed list
+          i->unlink();
           r_cache_size -= i->size;
-          unlink_cache_entry(i->path);
           removed_dirs += i->path + ", ";
         }
 
@@ -238,43 +304,29 @@ clean_cache(systemtap_session& s)
     }
 }
 
-//Get the size, in bytes, of the module (.ko) and the
-// corresponding source (.c)
-long
-get_cache_file_size(const string &cache_ent_path)
+// Get the size of a file in bytes
+static size_t
+get_file_size(const string &path)
 {
-  size_t cache_ent_size = 0;
-  string mod_path    = cache_ent_path + ".ko",
-         source_path = cache_ent_path + ".c";
-
   struct stat file_info;
 
-  if (stat(mod_path.c_str(), &file_info) == 0)
-    cache_ent_size += file_info.st_size;
+  if (stat(path.c_str(), &file_info) == 0)
+    return file_info.st_size;
   else
     return 0;
-
-  //Don't care if the .c isn't there, it's much smaller
-  // than the .ko anyway
-  if (stat(source_path.c_str(), &file_info) == 0)
-    cache_ent_size += file_info.st_size;
-
-
-  return cache_ent_size; // / 1024 / 1024;	//convert to MiB
 }
 
-//Assign a weight to this cache entry. A lower weight
+//Assign a weight for a particular file. A lower weight
 // will be removed before a higher weight.
 //TODO: for now use system mtime... later base a
 // weighting on size, ctime, atime etc..
-long
-get_cache_file_weight(const string &cache_ent_path)
+static long
+get_file_weight(const string &path)
 {
   time_t dir_mtime = 0;
   struct stat dir_stat_info;
-  string module_path = cache_ent_path + ".ko";
 
-  if (stat(module_path.c_str(), &dir_stat_info) == 0)
+  if (stat(path.c_str(), &dir_stat_info) == 0)
     //GNU struct stat defines st_atime as st_atim.tv_sec
     // but it doesnt seem to work properly in practice
     // so use st_atim.tv_sec -- bad for portability?
@@ -284,16 +336,36 @@ get_cache_file_weight(const string &cache_ent_path)
 }
 
 
-//deletes the module and source file contain
-void
-unlink_cache_entry(const string &cache_ent_path)
+cache_ent_info::cache_ent_info(const string& path, bool is_module):
+  path(path), is_module(is_module)
 {
-  //remove both .ko and .c files
-  string mod_path    = cache_ent_path + ".ko";
-  string source_path = cache_ent_path + ".c";
+  if (is_module)
+    {
+      string mod_path    = path + ".ko";
+      string source_path = path + ".c";
+      size = get_file_size(mod_path) + get_file_size(source_path);
+      weight = get_file_weight(mod_path);
+    }
+  else
+    {
+      size = get_file_size(path);
+      weight = get_file_weight(path);
+    }
+}
 
-  unlink(mod_path.c_str());     //it must exist, globbed for it earlier
-  unlink(source_path.c_str());  //if its not there, no matter
+
+void
+cache_ent_info::unlink() const
+{
+  if (is_module)
+    {
+      string mod_path    = path + ".ko";
+      string source_path = path + ".c";
+      ::unlink(mod_path.c_str());
+      ::unlink(source_path.c_str());
+    }
+  else
+    ::unlink(path.c_str());
 }
 
 /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */
diff --git a/cache.h b/cache.h
index 1a9827b7..89f44a43 100644
--- a/cache.h
+++ b/cache.h
@@ -1,22 +1,4 @@
-#define SYSTEMTAP_CACHE_MAX_FILENAME "cache_mb_limit"
-#define SYSTEMTAP_CACHE_DEFAULT_MB 64
-
-struct cache_ent_info {
-  std::string path;
-  size_t size;
-  long weight;  //lower == removed earlier
-};
-
-struct weight_sorter {
-  bool operator() (const struct cache_ent_info& c1, const struct cache_ent_info& c2) const
-  { return c1.weight < c2.weight;}
-};
-
 void add_to_cache(systemtap_session& s);
 bool get_from_cache(systemtap_session& s);
-void clean_cache(systemtap_session& s);
-long get_cache_file_size(const std::string &cache_ent_path);
-long get_cache_file_weight(const std::string &cache_ent_path);
-void unlink_cache_entry(const std::string &cache_ent_path);
 
 /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */
diff --git a/hash.cxx b/hash.cxx
index de8fd3d2..61caa356 100644
--- a/hash.cxx
+++ b/hash.cxx
@@ -223,7 +223,8 @@ find_stapconf_hash (systemtap_session& s, const hash& base)
   if (!create_hashdir(s, result, hashdir))
     return;
 
-  s.stapconf_path = hashdir + "/stapconf_" + result;
+  s.stapconf_name = "stapconf_" + result + ".h";
+  s.stapconf_path = hashdir + "/" + s.stapconf_name;
 }
 
 
diff --git a/main.cxx b/main.cxx
index 31ecda33..dbb2a306 100644
--- a/main.cxx
+++ b/main.cxx
@@ -355,6 +355,7 @@ main (int argc, char * const argv [])
   s.buffer_size = 0;
   s.last_pass = 5;
   s.module_name = "stap_" + stringify(getpid());
+  s.stapconf_name = "stapconf_" + stringify(getpid()) + ".h";
   s.output_file = ""; // -o FILE
   s.keep_tmpdir = false;
   s.cmd = "";
diff --git a/session.h b/session.h
index c55310d3..6bcebd8b 100644
--- a/session.h
+++ b/session.h
@@ -86,6 +86,7 @@ struct systemtap_session
   std::string runtime_path;
   std::string data_path;
   std::string module_name;
+  std::string stapconf_name;
   std::string output_file;
   std::string cmd;
   int target_pid;