Allowing user space file-system to (kind-of) keep more than maximum number of files open

Question

The linked code below is meant to be used in a user space file-system (fuse). Given that multiple processes may have multiple files opened on the file-system, the files on the specific user space file-system will map each of these open files to an (encrypted) file on a regular file-system, attempting to keep all mapped files open at the same time might fail pretty soon if there are many processes accessing the file-system. Doing an open->action->close on each file-system operation as an alternative would potentially have a rather big overhead.

The below container is meant to allow keeping a large set of files open while possible, hopefully the ones that are most likely to get once again accessed soon, and temporary low-level closing the ones that have been inactive for a while.

Is the design any good, or am I making bad assumptions that would result in trashing behaviour in real life usage scenarios? And if the design is OK, any other feedback on the code would be greatly appreciated.

template <typename nodeType,size_t maxOpenFiles,size_t maxQueueSize>
class openfilecollection {
    nodeType mNull;
    uint64_t mLastHandle;  
    std::map<uint64_t, nodeType> mCollection; 
    std::map<uint64_t, size_t> mFullyOpen; 
    std::deque<uint64_t> mOpperQue;
    uint64_t  getFreeFhNumber() {
       while ((mLastHandle == 0) || (mCollection.count(mLastHandle))) {
         mLastHandle += 1;
       }
       return mLastHandle;
    }     
    void cleanupQueFront() {
        while ((mFullyOpen.count(mOpperQue.front()) == 0)|| 
               (mFullyOpen[mOpperQue.front()]>1)||
               (mOpperQue.size()>maxQueueSize)) {
            if (mFullyOpen.count(mOpperQue.front()) != 0) {
              mFullyOpen[mOpperQue.front()] -= 1; 
              if (mFullyOpen[mOpperQue.front()] == 0) { 
                  mCollection[mOpperQue.front()].lowLevelClose();
                  mFullyOpen.erase(mOpperQue.front());
              }
            }
            mOpperQue.pop_front();
        }
    }
    void tempCloseIfNeeded() {
        while (mFullyOpen.size() > maxOpenFiles) {
            uint64_t candidate = mOpperQue.front(); 
            mOpperQue.pop_front(); 
            if (mFullyOpen.count(candidate)) { 
              mFullyOpen[candidate] -= 1;  
              if (mFullyOpen[candidate] == 0) { 
                  mCollection[candidate].lowLevelClose();
                  mFullyOpen.erase(candidate);
              }
            }
        }
        this->cleanupQueFront();
    }
    openfilecollection(openfilecollection const &) = delete;
    openfilecollection &operator=(openfilecollection const &) = delete;
  public:
    openfilecollection():mNull(),mLastHandle(0){} 
    ~openfilecollection() { 
       for (std::map<uint64_t, size_t>::iteratori1=mFullyOpen.begin();  
              i1 != mFullyOpen.end(); 
              ++i1) {
           uint64_t fh = i1->first;
           mCollection[fh].lowLevelClose();
       }
    }
    class node_handle {
        openfilecollection<nodeType, maxOpenFiles, maxQueueSize> *mCol;
        uint64_t mFh;
       public:
        node_handle(openfilecollection<nodeType, maxOpenFiles,maxQueueSize> *col, 
                                       uint64_t fh):mCol(col),mFh(fh){}
        void close() {
           mCol->close(mFh);
        }
        ssize_t read(void *buf, size_t count,off_t offset) {
           return mCol->mCollection[mFh].read(buf,count);
        }
        ssize_t write(const void *buf, size_t count,off_t offset) {
           return mCol->mCollection[mFh].write(buf,count);
        }
        int chmod(mode_t mode) {
           return mCol->mCollection[mFh].chmod(mode);
        }
    };
    node_handle operator[](uint64_t fh) {
        if (mCollection.count(fh) == 0) { /
            return node_handle(*this,0);
        }
        if (mFullyOpen.count(fh)) { 
          if (fh != mOpperQue.back()) { 
            mFullyOpen[fh] += 1;  
            mOpperQue.push_back(fh);
          }
        } else { 
          mFullyOpen[fh] = 1; 
          mOpperQue.push_back(fh); 
          this->tempCloseIfNeeded(); 
          mCollection[fh].lowLevelOpen(); 
        }
        return node_handle(this,fh); 
    }
    template<typename ... Args>
    uint64_t open(Args&& ... args) {
        uint64_t fh=this->getFreeFhNumber(); 
        mCollection.emplace(std::piecewise_construct, 
                            std::forward_as_tuple(fh),
                            std::forward_as_tuple(args...));
        mFullyOpen[fh] = 1; 
        mOpperQue.push_back(fh); 
        this->tempCloseIfNeeded(); 
        mCollection[fh].lowLevelOpen(); 
        return fh; 
    }
    void close(uint64_t fh) {
        if (mFullyOpen.count(fh)) {
            mFullyOpen.erase(fh);
            mCollection[fh].lowLevelClose();
        }
        if (mCollection.count(fh)) {
          mCollection.erase(fh);
        }
        this->cleanupQueFront();
    }
};

Here is a commented version of the above code

Answer 1

From a once-over:

Naming

• You seem to use all kinds of conventions, all lowercase, sometimes you use an underscore, sometimes you use lowerCamelCase. Perhaps you should stick to the underscore_style.

• The name mOpperQue is unfortunate, perhaps closing_candidates ?

• Could be me, but operator seems unfortunate as well, the name does not at all give away that it will actually open the file.
• tempCloseIfNeeded is unfortunate, I am not sure which part is temp. I would call it force_close_files.

DRY

• cleanupQueFront and tempCloseIfNeeded share enough code that you could either make it 1 function or extract the common code into a utility function.

• node_handle operator[](uint64_t fh) { and uint64_t open(Args&& ... args) { share the same code lines and comments, one more opportunity to merge or use a utility function.

Philosophy

• I would much rather have a library that says Sorry, you can't have this open file, than a library that "willy nilly" can close a file so that I have to keep checking whether my node_handle is still valid.

• By only checking the front for handlers that might need to be closed, you can create artificial limit problems, closing files that don't need to be closed.

• If you called tempCloseIfNeeded first and enforced (mFullyOpen.size() > maxOpenFiles - 1 ) and then call lowLevelOpen(), then you would never violate maxOpenFiles.