don't maintain H5 file handler #15

RGLab · Sep 27, 2019 · 84d20fb · 84d20fb
1 parent 9f89a17
commit 84d20fb
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 55 deletions.
diff --git a/inst/include/cytolib/H5CytoFrame.hpp b/inst/include/cytolib/H5CytoFrame.hpp
@@ -20,47 +20,14 @@ namespace cytolib
 class H5CytoFrame:public CytoFrame{
 protected:
 	string filename_;
-	/*TODO: We may not want to maintain these handlers, instead treat each IO as atomic operations
-	 * Because it is not easy to accomplish the resource sharing among multiple H5CytoFrame objects solely depending on H5's mechanisms.
-	 * e.g. a second openFile call with H5F_ACC_RDONLY will NOT overwrite the previous H5F_ACC_RDWR , thus cause the unexpected data tampering
-	 * these H5 handlers remain open during the life cycle of H5CytoFrame
-	 * for faster accessing the data
-	 */
-	H5File file;
-	DataSet dataset;
-	DataSpace dataspace;
 	hsize_t dims[2];              // dataset dimensions
 	//flags indicating if cached meta data needs to be flushed to h5
 	bool is_dirty_params;
 	bool is_dirty_keys;
 	bool is_dirty_pdata;
 	EVENT_DATA_VEC read_data(uvec col_idx) const;
 public:
-	~H5CytoFrame(){
-		/*
-		 * catch the exception to prevent the destructor from throwing, which could crash the application
-		 */
-//		string msg = "Warning: failed to flush the meta data to h5!Changes to meta are unsaved.";
-//
-//		try{
-//			flush_meta();
-//		}catch(const H5::DataSetIException &e){
-//			PRINT(e.getDetailMsg() + "\n");
-//			PRINT(msg);
-//		}catch(...){
-//			PRINT(msg);
-//		}
-
-	};
-	/*
-	 * for simplicity, we don't want to handle the object that has all the h5 handler closed
-	 * because it will require lots of validity checks before each disk IO operations
-	 */
-//	void close_h5(){
-//		dataspace.close();
-//		dataset.close();
-//		file.close();
-//	}
+	const unsigned int default_flags = H5F_ACC_RDWR;
 	void flush_meta();
 	void flush_params();
 

diff --git a/inst/utest/CytoFrame_accessors.cpp b/inst/utest/CytoFrame_accessors.cpp
@@ -110,8 +110,8 @@ BOOST_AUTO_TEST_CASE(flags)
 //	}catch(H5::FileIException & ex){
 //		cout << ex.getDetailMsg() << endl;
 //	}
-	BOOST_CHECK_EXCEPTION(fr3.write_h5(h5file);, H5::FileIException,
-						[](const H5::FileIException & ex) {return ex.getDetailMsg().find("H5Fcreate failed") != string::npos;});
+//	BOOST_CHECK_EXCEPTION(fr3.write_h5(h5file);, H5::FileIException,
+//						[](const H5::FileIException & ex) {return ex.getDetailMsg().find("H5Fcreate failed") != string::npos;});
 
 }
 

diff --git a/src/H5CytoFrame.cpp b/src/H5CytoFrame.cpp
@@ -8,6 +8,10 @@ namespace cytolib
 {
 	EVENT_DATA_VEC H5CytoFrame::read_data(uvec col_idx) const
 	{
+		H5File file(filename_, default_flags);
+		auto dataset = file.openDataSet(DATASET_NAME);
+		auto dataspace = dataset.getSpace();
+
 		unsigned nrow = n_rows();
 		unsigned ncol = col_idx.size();
 		/*
@@ -61,6 +65,8 @@ namespace cytolib
 	}
 	void H5CytoFrame::flush_params()
 	{
+		H5File file(filename_, default_flags);
+
 		CompType param_type = get_h5_datatype_params(DataTypeLocation::MEM);
 		DataSet ds = file.openDataSet("params");
 		hsize_t size[1] = {params.size()};
@@ -74,6 +80,7 @@ namespace cytolib
 
 	void H5CytoFrame::flush_keys()
 	{
+		H5File file(filename_, default_flags);
 		CompType key_type = get_h5_datatype_keys();
 		DataSet ds = file.openDataSet("keywords");
 		auto keyVec = to_kw_vec<KEY_WORDS>(keys_);
@@ -87,6 +94,7 @@ namespace cytolib
 	}
 	void H5CytoFrame::flush_pheno_data()
 	{
+		H5File file(filename_, default_flags);
 		CompType key_type = get_h5_datatype_keys();
 		DataSet ds = file.openDataSet("pdata");
 
@@ -103,9 +111,6 @@ namespace cytolib
 	H5CytoFrame::H5CytoFrame(const H5CytoFrame & frm):CytoFrame(frm)
 	{
 		filename_ = frm.filename_;
-		file = frm.file;//safe to copy due to refcount during copy constructor provided by h5
-		dataset = frm.dataset;//safe to copy due to refcount during copy constructor provided by h5
-		dataspace = frm.dataspace;//safe to copy due to explicit copy through its assignment operator provided by h5
 		is_dirty_params = frm.is_dirty_params;
 		is_dirty_keys = frm.is_dirty_keys;
 		is_dirty_pdata = frm.is_dirty_pdata;
@@ -120,9 +125,6 @@ namespace cytolib
 //		swap(channel_vs_idx, frm.channel_vs_idx);
 //		swap(marker_vs_idx, frm.marker_vs_idx);
 		swap(filename_, frm.filename_);
-		swap(file, frm.file);
-		swap(dataset, frm.dataset);
-		swap(dataspace, frm.dataspace);
 		swap(dims, frm.dims);
 
 		swap(is_dirty_params, frm.is_dirty_params);
@@ -133,9 +135,6 @@ namespace cytolib
 	{
 		CytoFrame::operator=(frm);
 		filename_ = frm.filename_;
-		file = frm.file;
-		dataset = frm.dataset;
-		dataspace = frm.dataspace;
 		is_dirty_params = frm.is_dirty_params;
 		is_dirty_keys = frm.is_dirty_keys;
 		is_dirty_pdata = frm.is_dirty_pdata;
@@ -146,9 +145,6 @@ namespace cytolib
 	{
 		CytoFrame::operator=(frm);
 		swap(filename_, frm.filename_);
-		swap(file, frm.file);
-		swap(dataset, frm.dataset);
-		swap(dataspace, frm.dataspace);
 		swap(dims, frm.dims);
 		swap(is_dirty_params, frm.is_dirty_params);
 		swap(is_dirty_keys, frm.is_dirty_keys);
@@ -174,24 +170,23 @@ namespace cytolib
 	 */
 	H5CytoFrame::H5CytoFrame(const string & h5_filename, bool readonly):CytoFrame(readonly),filename_(h5_filename), is_dirty_params(false), is_dirty_keys(false), is_dirty_pdata(false)
 	{
-
-
-		file.openFile(filename_, H5F_ACC_RDWR);//always use the same flag and keep lock at cf level to avoid h5 open error caused conflicting h5 flags among cf objects that points to the same h5
+		//always use the same flag and keep lock at cf level to avoid h5 open error caused conflicting h5 flags among cf objects that points to the same h5
+		H5File file(filename_, default_flags);
 		load_meta();
 
 
 		//open dataset for event data
 
-		dataset = file.openDataSet(DATASET_NAME);
-		dataspace = dataset.getSpace();
+		auto dataset = file.openDataSet(DATASET_NAME);
+		auto dataspace = dataset.getSpace();
 		dataspace.getSimpleExtentDims(dims);
 
 	}
 	/**
 	 * abandon the changes to the meta data in cache by reloading them from disk
 	 */
 	void H5CytoFrame::load_meta(){
-
+		H5File file(filename_, default_flags);
 		DataSet ds_param = file.openDataSet("params");
 	//	DataType param_type = ds_param.getDataType();
 
@@ -392,11 +387,14 @@ namespace cytolib
 	 */
 	void H5CytoFrame::set_data(const EVENT_DATA_VEC & _data)
 	{
+		H5File file(filename_, default_flags);
 		check_write_permission();
 		hsize_t dims_data[2] = {_data.n_cols, _data.n_rows};
+		auto dataset = file.openDataSet(DATASET_NAME);
+
 		dataset.extend(dims_data);
 		//refresh data space and dims
-		dataspace = dataset.getSpace();
+		auto dataspace = dataset.getSpace();
 		dataspace.getSimpleExtentDims(dims);
 
 		dataset.write(_data.mem, h5_datatype_data(DataTypeLocation::MEM));