changeset 395:bc7a821004bb api-inversion

Invert audioDB::status / audiodb_status().

To do that without breaking abstractions, we actually need a new field
in the status structure, storing the size of the data region.
Previously, this was computed in the audioDB::status request from the
database header, but I'm assuming that "user" code doesn't have access
to such internals.  While we're at it, name some intermediate values in
audioDB::status() so that I don't get confused.

Here's the thing, though: we need to make sure that the adb_t * that we
have from audiodb_open() or audiodb_create() is propagated all the way
through into the C++ routines that implement library functions -- in
particular those which actually write to the database; otherwise we
won't have a consistent view in memory of the header on-disk (as the adb
header that will have been written to disk won't be the same as the one
in memory).

We can do that, by altering the "API" audioDB constructors to take the
adb_t * argument, and setting the adb field in the audioDB object that
we've already introduced to that.  But now we need to be careful a
couple of times: if we have one, then audioDB::initTables() mustn't
stomp on it; also, if we're only constructing an audioDB instance to
fulfil an API request, we mustn't audiodb_close() the one we have when
we destroy the audioDB object, because the adb_t * is the one we have
passed in and are going to reuse in later calls to the API.

The good news is that we can be careful in just these ways with minimal
code.  The really good news is that once the inversion is complete, all
of this horribleness will automatically go away (as there will be no
code which constructs audioDB objects to fulfil API functions).  Hooray!
It's almost like it was all planned this way.

--- a/Makefile	Tue Nov 25 16:41:00 2008 +0000
+++ b/Makefile	Tue Nov 25 16:41:01 2008 +0000
@@ -8,7 +8,7 @@
 
 SHARED_LIB_FLAGS=-shared -Wl,-soname,
 
-LIBOBJS=insert.o create.o common.o open.o close.o dump.o liszt.o query.o sample.o index.o lshlib.o cmdline.o
+LIBOBJS=insert.o create.o common.o open.o close.o status.o dump.o liszt.o query.o sample.o index.o lshlib.o cmdline.o
 OBJS=$(LIBOBJS) soap.o audioDB.o
 
 

--- a/audioDB.cpp	Tue Nov 25 16:41:00 2008 +0000
+++ b/audioDB.cpp	Tue Nov 25 16:41:01 2008 +0000
@@ -166,11 +166,12 @@
 
 
 //for the lib / API
-audioDB::audioDB(const unsigned argc, const char *argv[], int * apierror): O2_AUDIODB_INITIALIZERS
+audioDB::audioDB(const unsigned argc, const char *argv[], int * apierror, adb_t *a): O2_AUDIODB_INITIALIZERS
 {
 
     try {
         UseApiError=1;
+	adb = a;
 
         if(processArgs(argc, argv)<0){
             printf("No command found.\n");
@@ -230,46 +231,13 @@
 
 }
 
-//for API status
-audioDB::audioDB(const unsigned argc, const char *argv[], adb_status_t *stat, int * apierror): O2_AUDIODB_INITIALIZERS
+//for API query
+audioDB::audioDB(const unsigned argc, const char *argv[],adb__queryResponse *adbQueryResponse, int * apierror, adb_t *a): O2_AUDIODB_INITIALIZERS
 {
 
     try {
         UseApiError=1;
-
-
-        if(processArgs(argc, argv)<0){
-            printf("No command found.\n");
-            cmdline_parser_print_version ();
-            if (strlen(gengetopt_args_info_purpose) > 0)
-                printf("%s\n", gengetopt_args_info_purpose);
-            printf("%s\n", gengetopt_args_info_usage);
-            printf("%s\n", gengetopt_args_info_help[1]);
-            printf("%s\n", gengetopt_args_info_help[2]);
-            printf("%s\n", gengetopt_args_info_help[0]);
-            error("No command found");
-        }
-
-        status(dbName, stat);
-
-
-    } catch(int a) {
-        *apierror=a;
-        return;
-
-    }
-    *apierror=apierrortemp;
-    return;
-
-}
-
-
-//for API query
-audioDB::audioDB(const unsigned argc, const char *argv[],adb__queryResponse *adbQueryResponse, int * apierror): O2_AUDIODB_INITIALIZERS
-{
-
-    try {
-        UseApiError=1;
+	adb = a;
 
         if(processArgs(argc, argv)<0){
             printf("No command found.\n");
@@ -295,10 +263,6 @@
 
 }
 
-
-
-
-
 void audioDB::cleanup() {
   cmdline_parser_free(&args_info);
   if(indata)
@@ -335,7 +299,7 @@
     delete vv;
   if(infid>0)
     close(infid);
-  if(adb) {
+  if(adb && !UseApiError) {
     audiodb_close(adb);
     adb = NULL;
   }
@@ -742,83 +706,56 @@
 }
 
 void audioDB::status(const char* dbName, adb__statusResponse *adbStatusResponse){
-  if(!dbH)
-    initTables(dbName, 0);
-
-  unsigned dudCount=0;
-  unsigned nullCount=0;
-  for(unsigned k=0; k<dbH->numFiles; k++){
-    if(trackTable[k]<sequenceLength){
-      dudCount++;
-      if(!trackTable[k])
-        nullCount++;
-    }
+  adb_t *adb;
+  adb_status_t status;
+  if(!(adb = audiodb_open(dbName, O_RDONLY))) {
+    error("Failed to open database file", dbName);
   }
+  if(audiodb_status(adb, &status)) {
+    error("Failed to retrieve database status", dbName);
+  }
+  audiodb_close(adb);
   
   if(adbStatusResponse == 0) {
-
-    // Update Header information
-    std::cout << "num files:" << dbH->numFiles << std::endl;
-    std::cout << "data dim:" << dbH->dim <<std::endl;
-    if(dbH->dim>0){
-      std::cout << "total vectors:" << dbH->length/(sizeof(double)*dbH->dim)<<std::endl;
-      if(dbH->flags & O2_FLAG_LARGE_ADB)
-	std::cout << "vectors available:" << O2_MAX_VECTORS - (dbH->length / (sizeof(double)*dbH->dim)) << std::endl;
-      else
-	std::cout << "vectors available:" << (dbH->timesTableOffset-(dbH->dataOffset+dbH->length))/(sizeof(double)*dbH->dim) << std::endl;
+    size_t bytes_per_vector = sizeof(double) * status.dim;
+    off_t nvectors = status.length / bytes_per_vector;
+    off_t data_region_vectors = status.data_region_size / bytes_per_vector;
+    std::cout << "num files:" << status.numFiles << std::endl;
+    std::cout << "data dim:" << status.dim <<std::endl;
+    if(status.dim > 0) {
+      std::cout << "total vectors:" << nvectors << std::endl;
+      std::cout << "vectors available:";
+      if(status.flags & O2_FLAG_LARGE_ADB) {
+	std::cout << O2_MAX_VECTORS - nvectors << std::endl;
+      } else {
+	std::cout << data_region_vectors - nvectors << std::endl;
+      }
     }
-    if( ! (dbH->flags & O2_FLAG_LARGE_ADB) ){
-      std::cout << "total bytes:" << dbH->length << " (" << (100.0*dbH->length)/(dbH->timesTableOffset-dbH->dataOffset) << "%)" << std::endl;
-      std::cout << "bytes available:" << dbH->timesTableOffset-(dbH->dataOffset+dbH->length) << " (" <<
-	(100.0*(dbH->timesTableOffset-(dbH->dataOffset+dbH->length)))/(dbH->timesTableOffset-dbH->dataOffset) << "%)" << std::endl;
+    if(!(status.flags & O2_FLAG_LARGE_ADB)) {
+      double used_frac = ((double) status.length) / status.data_region_size;
+      std::cout << "total bytes:" << status.length << 
+	" (" << (100.0*used_frac) << "%)" << std::endl;
+      std::cout << "bytes available:" << status.data_region_size - status.length << 
+	" (" << (100.0*(1-used_frac)) << "%)" << std::endl;
     }
-    std::cout << "flags:" << " l2norm[" << DISPLAY_FLAG(dbH->flags&O2_FLAG_L2NORM)
-	      << "] minmax[" << DISPLAY_FLAG(dbH->flags&O2_FLAG_MINMAX)
-	      << "] power[" << DISPLAY_FLAG(dbH->flags&O2_FLAG_POWER)
-	      << "] times[" << DISPLAY_FLAG(dbH->flags&O2_FLAG_TIMES) 
-	      << "] largeADB[" << DISPLAY_FLAG(dbH->flags&O2_FLAG_LARGE_ADB)
+    std::cout << "flags:" << " l2norm[" << DISPLAY_FLAG(status.flags&O2_FLAG_L2NORM)
+	      << "] minmax[" << DISPLAY_FLAG(status.flags&O2_FLAG_MINMAX)
+	      << "] power[" << DISPLAY_FLAG(status.flags&O2_FLAG_POWER)
+	      << "] times[" << DISPLAY_FLAG(status.flags&O2_FLAG_TIMES) 
+	      << "] largeADB[" << DISPLAY_FLAG(status.flags&O2_FLAG_LARGE_ADB)
 	      << "]" << endl;    
               
-    std::cout << "null count: " << nullCount << " small sequence count " << dudCount-nullCount << std::endl;    
+    std::cout << "null count: " << status.nullCount << " small sequence count " << status.dudCount-status.nullCount << std::endl;    
   } else {
-    adbStatusResponse->result.numFiles = dbH->numFiles;
-    adbStatusResponse->result.dim = dbH->dim;
-    adbStatusResponse->result.length = dbH->length;
-    adbStatusResponse->result.dudCount = dudCount;
-    adbStatusResponse->result.nullCount = nullCount;
-    adbStatusResponse->result.flags = dbH->flags;
+    adbStatusResponse->result.numFiles = status.numFiles;
+    adbStatusResponse->result.dim = status.dim;
+    adbStatusResponse->result.length = status.length;
+    adbStatusResponse->result.dudCount = status.dudCount;
+    adbStatusResponse->result.nullCount = status.nullCount;
+    adbStatusResponse->result.flags = status.flags;
   }
 }
 
-///used by lib/API
-void audioDB::status(const char* dbName, adb_status_t *status){
-    if(!dbH) {
-        initTables(dbName, 0);
-    }
-
-  unsigned dudCount=0;
-  unsigned nullCount=0;
-  for(unsigned k=0; k<dbH->numFiles; k++){
-    if(trackTable[k]<sequenceLength){
-      dudCount++;
-      if(!trackTable[k])
-        nullCount++;
-    }
-  }
-  
-  status->numFiles = dbH->numFiles;
-  status->dim = dbH->dim;
-  status->length = dbH->length;
-  status->dudCount = dudCount;
-  status->nullCount = nullCount;
-  status->flags = dbH->flags;
-  
-  return;
-}
-
-
-
-
 void audioDB::l2norm(const char* dbName) {
   forWrite = true;
   initTables(dbName, 0);
@@ -944,7 +881,7 @@
     }
     argv[argvctr]='\0';
 
-    audioDB::audioDB(argvctr,argv,&apierror);
+    audioDB::audioDB(argvctr,argv,&apierror,mydb);
     return apierror;
   }
 
@@ -1051,7 +988,7 @@
 
     argv[argvctr]='\0';
 
-    audioDB::audioDB(argvctr,argv,&apierror);
+    audioDB::audioDB(argvctr,argv,&apierror,mydb);
 
     remove(tempfeaturename);
     remove(temppowername);
@@ -1176,7 +1113,7 @@
 
     /* debugging */
 
-    audioDB::audioDB(argvctr,argv, &adbQueryResponse, &apierror);
+    audioDB::audioDB(argvctr,argv, &adbQueryResponse, &apierror,mydb);
 
     //copy data over here from adbQueryResponse to adbqr
     adbqr->sizeRlist=adbQueryResponse.result.__sizeRlist;
@@ -1191,24 +1128,6 @@
     return apierror;
   }
 
-  ///* status command */
-  int audiodb_status(adb_ptr mydb, adb_status_ptr status){
-      int apierror=0;
-
-      const char *argv[5];
-
-      apierror=0;
-      argv[0]="audioDB";
-      argv[1]="--STATUS";
-      argv[2]="-d";
-      argv[3]=mydb->path;
-      argv[4]='\0';
-
-      audioDB::audioDB(4,argv,status ,&apierror);
-      
-      return apierror;
-  }
-
   int audiodb_dump(adb_ptr mydb){
       return audiodb_dump_withdir(mydb,"audioDB.dump");
   }
@@ -1227,7 +1146,7 @@
       argv[argvctr++]=(char *)outputdir;
       argv[argvctr]='\0';
 
-      audioDB::audioDB(6,argv,&apierror);
+      audioDB::audioDB(6,argv,&apierror,mydb);
 
       return apierror;
   }
@@ -1243,7 +1162,7 @@
       argv[3]=mydb->path;
       argv[4]='\0';
 
-      audioDB::audioDB(4,argv,&apierror);
+      audioDB::audioDB(4,argv,&apierror,mydb);
       return apierror;
   }
 
@@ -1258,7 +1177,7 @@
       argv[3]=mydb->path;
       argv[4]='\0';
 
-      audioDB::audioDB(4,argv,&apierror);
+      audioDB::audioDB(4,argv,&apierror,mydb);
       return apierror;
   }
 }

--- a/audioDB.h	Tue Nov 25 16:41:00 2008 +0000
+++ b/audioDB.h	Tue Nov 25 16:41:01 2008 +0000
@@ -347,9 +347,8 @@
   audioDB(const unsigned argc, const char *argv[], adb__queryResponse *adbQueryResponse);
   audioDB(const unsigned argc, const char *argv[], adb__statusResponse *adbStatusResponse);
   audioDB(const unsigned argc, const char *argv[], adb__lisztResponse *adbLisztResponse);
-  audioDB(const unsigned argc, const char *argv[], int * apierror);
-  audioDB(const unsigned argc, const char *argv[], struct adbstatus *status, int * apierror);
-  audioDB(const unsigned argc, const char *argv[],adb__queryResponse *adbQueryResponse, int * apierror);
+  audioDB(const unsigned argc, const char *argv[], int * apierror, struct adb *a);
+  audioDB(const unsigned argc, const char *argv[],adb__queryResponse *adbQueryResponse, int * apierror, struct adb *a);
 
 
   void cleanup();
@@ -366,7 +365,6 @@
   void batchinsert_large_adb(const char* dbName, const char* inFile);
   void query(const char* dbName, const char* inFile, adb__queryResponse *adbQueryResponse=0);
   void status(const char* dbName, adb__statusResponse *adbStatusResponse=0);
-  void status(const char* dbName, struct adbstatus *status);
 
   unsigned random_track(unsigned *propTable, unsigned total);
   void sample(const char *dbName);

--- a/audioDB_API.h	Tue Nov 25 16:41:00 2008 +0000
+++ b/audioDB_API.h	Tue Nov 25 16:41:01 2008 +0000
@@ -38,14 +38,13 @@
 
 /* struct for returning status results */
 struct adbstatus {
-
     unsigned int numFiles;  
     unsigned int dim;
     unsigned int length;
     unsigned int dudCount;
     unsigned int nullCount;
     unsigned int flags;
-
+    off_t data_region_size;
 };
 typedef struct adbstatus adb_status_t, *adb_status_ptr;
 

--- a/common.cpp	Tue Nov 25 16:41:00 2008 +0000
+++ b/common.cpp	Tue Nov 25 16:41:01 2008 +0000
@@ -98,9 +98,11 @@
 }
 
 void audioDB::initDBHeader(const char* dbName) {
-  adb = audiodb_open(dbName, forWrite ? O_RDWR : O_RDONLY);
   if(!adb) {
-    error("Failed to open database", dbName);
+    adb = audiodb_open(dbName, forWrite ? O_RDWR : O_RDONLY);
+    if(!adb) {
+      error("Failed to open database", dbName);
+    }
   }
   dbfid = adb->fd;
   dbH = adb->header;

--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/status.cpp	Tue Nov 25 16:41:01 2008 +0000
@@ -0,0 +1,64 @@
+#include "audioDB.h"
+extern "C" {
+#include "audioDB_API.h"
+}
+
+int audiodb_status(adb_t *adb, adb_status_t *status) {
+  /* FIXME: it would be nice to be able to test for "is this database
+     pointer valid", but at the moment we punt that to memory
+     discipline.  */
+
+  unsigned dudCount = 0;
+  unsigned nullCount = 0;
+
+  off_t trackTableLength = ALIGN_PAGE_UP(adb->header->numFiles * O2_TRACKTABLE_ENTRY_SIZE);
+  unsigned *trackTable = 0;
+  void *tmp = 0;
+  if (adb->header->length > 0)  {
+    tmp = mmap(0, trackTableLength, PROT_READ, MAP_SHARED, adb->fd, adb->header->trackTableOffset);
+    if (tmp == (void *) -1) {
+      return 1;
+    }
+    trackTable = (unsigned *) tmp;
+  }
+
+  for(unsigned k = 0; k < adb->header->numFiles; k++) {
+    /* FIXME: this bare "16" here reveals a problem (or maybe two).
+     * 16 here means the default value of the sequenceLength parameter
+     * initializer (both in C++ and corresponding to the "-l" or
+     * "--sequencelength" command-line argument).
+     *
+     * The problem is that the API as currently designed provides no
+     * way to pass that information in to this routine; there's no
+     * input parameter; nor is there in the SOAP version of this
+     * query.  However, there /is/ a way to pass that information on
+     * the command-line -- though that codepath is completely
+     * untested.  I can see that it might be useful to provide this
+     * information, but at present it's probably completely unused, so
+     * the compromise for now is to hardwire the 16.
+     */ 
+    if(trackTable[k] < 16) {
+      dudCount++; 
+      if(!trackTable[k]) {
+	nullCount++; 
+      } 
+    }
+  }
+
+  if(adb->header->length > 0) {
+    if(munmap(trackTable, trackTableLength)) {
+      return 1;
+    }
+  }
+
+  status->numFiles = adb->header->numFiles;
+  status->dim = adb->header->dim;
+  status->length = adb->header->length;
+  status->dudCount = dudCount;
+  status->nullCount = nullCount;
+  status->flags = adb->header->flags;
+  status->data_region_size = adb->header->timesTableOffset - adb->header->dataOffset;
+
+  return 0;
+}
+