Skip to content

Commit

Permalink
BUG/ENH: Atomic File Rework (#900)
Browse files Browse the repository at this point in the history
- Removed Autocommit
- Added Additional Documentation
- Revamped Atomic File code to be safer and consolidated
- Updated multiple filters that use Atomic File to cut down extraneous copies of potentially large result objects
- Defined new writer code paradigm that makes heavy use of error checking at multiple stages
- Updated all existing writers to reflect the new paradigm
  • Loading branch information
nyoungbq authored Apr 5, 2024
1 parent 9d96f63 commit d1b4b50
Show file tree
Hide file tree
Showing 24 changed files with 500 additions and 212 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,6 @@ class Converter
, m_SqrYStep(spacingXY.at(1))
{
}
~Converter() noexcept
{
if(m_Valid)
{
for(const auto& atomicFile : m_AtomicFiles)
{
atomicFile->commit();
}
}
}

Converter(const Converter&) = delete; // Copy Constructor Default Implemented
Converter(Converter&&) = delete; // Move Constructor Not Implemented
Expand Down Expand Up @@ -81,7 +71,12 @@ class Converter
}

std::istringstream headerStream(origHeader, std::ios_base::in | std::ios_base::binary);
m_AtomicFiles.emplace_back(std::make_unique<AtomicFile>((fs::absolute(m_OutputPath) / (m_FilePrefix + inputPath.filename().string())), false));
m_AtomicFiles.emplace_back(std::make_unique<AtomicFile>((fs::absolute(m_OutputPath) / (m_FilePrefix + inputPath.filename().string()))));
auto creationResult = m_AtomicFiles[m_Index]->getResult();
if(creationResult.invalid())
{
return creationResult;
}
fs::path outPath = m_AtomicFiles[m_Index]->tempFilePath();

// Ensure the output path exists by creating it if necessary
Expand Down Expand Up @@ -224,6 +219,26 @@ class Converter
return {};
}

Result<> commitAllFiles()
{
if(m_Valid)
{
for(const auto& atomicFile : m_AtomicFiles)
{
if(!atomicFile->commit())
{
return atomicFile->getResult();
}
}
}
else
{
return MakeErrorResult(-77751, "The files were invalidated during execution, and this point should not be reached without an error result. If this is the sole error please make a bug report by "
"checking the repository at the bottom of this filters documentation!");
}
return {};
}

private:
const std::atomic_bool& m_ShouldCancel;
const fs::path& m_OutputPath;
Expand Down Expand Up @@ -338,7 +353,9 @@ Result<> ConvertHexGridToSquareGrid::operator()()
{
if(!m_InputValues->MultiFile)
{
return ::Converter(getCancel(), m_InputValues->OutputPath, m_InputValues->OutputFilePrefix, m_InputValues->XYSpacing)(m_InputValues->InputPath);
auto converter = ::Converter(getCancel(), m_InputValues->OutputPath, m_InputValues->OutputFilePrefix, m_InputValues->XYSpacing);
converter(m_InputValues->InputPath);
return converter.commitAllFiles();
}

// Now generate all the file names the user is asking for and populate the table
Expand Down Expand Up @@ -405,5 +422,7 @@ Result<> ConvertHexGridToSquareGrid::operator()()
}
}

result = MergeResults(converter.commitAllFiles(), result);

return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,11 @@ Result<> EbsdToH5Ebsd::operator()()
}
}

AtomicFile atomicFile(absPath.string(), false);

auto dirResult = atomicFile.getResult();
if(dirResult.invalid())
AtomicFile atomicFile(absPath.string());
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return dirResult;
return creationResult;
}

// Scope file writer in code block to get around file lock on windows (enforce destructor order)
Expand Down Expand Up @@ -323,6 +322,9 @@ Result<> EbsdToH5Ebsd::operator()()
}
}

atomicFile.commit();
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
return {};
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,12 @@ IFilter::PreflightResult WriteGBCDGMTFileFilter::preflightImpl(const DataStructu
Result<> WriteGBCDGMTFileFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
const std::atomic_bool& shouldCancel) const
{
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key), true);
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}

WriteGBCDGMTFileInputValues inputValues;

Expand All @@ -135,7 +140,13 @@ Result<> WriteGBCDGMTFileFilter::executeImpl(DataStructure& dataStructure, const
inputValues.CrystalStructuresArrayPath = filterArgs.value<DataPath>(k_CrystalStructuresArrayPath_Key);

auto result = WriteGBCDGMTFile(dataStructure, messageHandler, shouldCancel, &inputValues)();
atomicFile.setAutoCommit(result.valid());
if(result.valid())
{
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ IFilter::PreflightResult WriteGBCDTriangleDataFilter::preflightImpl(const DataSt
Result<> WriteGBCDTriangleDataFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
const std::atomic_bool& shouldCancel) const
{
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key), true);
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}

WriteGBCDTriangleDataInputValues inputValues;

Expand All @@ -121,7 +126,13 @@ Result<> WriteGBCDTriangleDataFilter::executeImpl(DataStructure& dataStructure,
inputValues.FeatureEulerAnglesArrayPath = filterArgs.value<DataPath>(k_FeatureEulerAnglesArrayPath_Key);

auto result = WriteGBCDTriangleData(dataStructure, messageHandler, shouldCancel, &inputValues)();
atomicFile.setAutoCommit(result.valid());
if(result.valid())
{
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ IFilter::PreflightResult WriteINLFileFilter::preflightImpl(const DataStructure&
Result<> WriteINLFileFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
const std::atomic_bool& shouldCancel) const
{
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key), true);
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}

WriteINLFileInputValues inputValues;

Expand All @@ -125,7 +130,13 @@ Result<> WriteINLFileFilter::executeImpl(DataStructure& dataStructure, const Arg
inputValues.NumFeaturesArrayPath = filterArgs.value<DataPath>(k_NumFeaturesArrayPath_Key);

auto result = WriteINLFile(dataStructure, messageHandler, shouldCancel, &inputValues)();
atomicFile.setAutoCommit(result.valid());
if(result.valid())
{
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,15 @@ Result<> WriteAbaqusHexahedron::operator()()
fileList.push_back(std::make_unique<AtomicFile>(m_InputValues->OutputPath.string() + "/" + m_InputValues->FilePrefix + "_elset.inp"));
fileList.push_back(std::make_unique<AtomicFile>(m_InputValues->OutputPath.string() + "/" + m_InputValues->FilePrefix + ".inp"));

for(auto& file : fileList)
{
auto creationResult = file->getResult();
if(creationResult.invalid())
{
return creationResult;
}
}

int32 err = writeNodes(this, fileList[0]->tempFilePath().string(), cDims.data(), origin.data(), spacing.data(), getCancel()); // Nodes file
if(err < 0)
{
Expand Down Expand Up @@ -432,7 +441,10 @@ Result<> WriteAbaqusHexahedron::operator()()

for(auto& file : fileList)
{
file->commit();
if(!file->commit())
{
return file->getResult();
}
}

return {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ Result<> WriteStlFile::operator()()

if(groupingType == GroupingType::None)
{
AtomicFile atomicFile(m_InputValues->OutputStlFile.string(), false);

if(atomicFile.getResult().invalid())
AtomicFile atomicFile(m_InputValues->OutputStlFile.string());
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return atomicFile.getResult();
return creationResult;
}

{ // Scoped to ensure file lock is released and header string is untouched since it is invalid after move
Expand All @@ -287,7 +287,10 @@ Result<> WriteStlFile::operator()()
}
}

atomicFile.commit();
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
return {};
}

Expand Down Expand Up @@ -319,12 +322,12 @@ Result<> WriteStlFile::operator()()
for(const auto featureId : uniqueGrainIds)
{
// Generate the output file
fileList.push_back(
std::make_unique<AtomicFile>(m_InputValues->OutputStlDirectory.string() + "/" + m_InputValues->OutputStlPrefix + "Feature_" + StringUtilities::number(featureId) + ".stl", false));
fileList.push_back(std::make_unique<AtomicFile>(m_InputValues->OutputStlDirectory.string() + "/" + m_InputValues->OutputStlPrefix + "Feature_" + StringUtilities::number(featureId) + ".stl"));

if(fileList[fileIndex]->getResult().invalid())
auto creationResult = fileList[fileIndex]->getResult();
if(creationResult.invalid())
{
return fileList[fileIndex]->getResult();
return creationResult;
}

m_MessageHandler(IFilter::Message::Type::Info, fmt::format("Writing STL for Feature Id {}", featureId));
Expand Down Expand Up @@ -357,12 +360,12 @@ Result<> WriteStlFile::operator()()
{
// Generate the output file
fileList.push_back(std::make_unique<AtomicFile>(m_InputValues->OutputStlDirectory.string() + "/" + m_InputValues->OutputStlPrefix + "Ensemble_" + StringUtilities::number(value) + "_" +
"Feature_" + StringUtilities::number(featureId) + ".stl",
false));
"Feature_" + StringUtilities::number(featureId) + ".stl"));

if(fileList[fileIndex]->getResult().invalid())
auto creationResult = fileList[fileIndex]->getResult();
if(creationResult.invalid())
{
return fileList[fileIndex]->getResult();
return creationResult;
}

m_MessageHandler(IFilter::Message::Type::Info, fmt::format("Writing STL for Feature Id {}", featureId));
Expand All @@ -383,7 +386,10 @@ Result<> WriteStlFile::operator()()

for(const auto& atomicFile : fileList)
{
atomicFile->commit();
if(!atomicFile->commit())
{
return atomicFile->getResult();
}
}

return {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ Result<> ExtractPipelineToFileFilter::executeImpl(DataStructure& dataStructure,
{
outputFile.replace_extension(extension);
}
AtomicFile atomicFile(outputFile.string(), false);
AtomicFile atomicFile(outputFile.string());
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}
{
const fs::path exportFilePath = atomicFile.tempFilePath();
std::ofstream fOut(exportFilePath.string(), std::ofstream::out); // test name resolution and create file
Expand All @@ -140,7 +145,11 @@ Result<> ExtractPipelineToFileFilter::executeImpl(DataStructure& dataStructure,

fOut << pipelineJson.dump(2);
}
atomicFile.commit();
if(!atomicFile.commit())
{
return atomicFile.getResult();
}

return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@ Result<> WriteASCIIDataFilter::executeImpl(DataStructure& dataStructure, const A

if(static_cast<WriteASCIIDataFilter::OutputStyle>(fileType) == WriteASCIIDataFilter::OutputStyle::SingleFile)
{
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputPath_Key), false);
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputPath_Key));
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}

auto outputPath = atomicFile.tempFilePath();
// Make sure any directory path is also available as the user may have just typed
// in a path without actually creating the full path
Expand All @@ -195,7 +201,11 @@ Result<> WriteASCIIDataFilter::executeImpl(DataStructure& dataStructure, const A

OStreamUtilities::PrintDataSetsToSingleFile(outStrm, selectedDataArrayPaths, dataStructure, messageHandler, shouldCancel, delimiter, includeIndex, includeHeaders);
}
atomicFile.setAutoCommit(true);

if(!atomicFile.commit())
{
return atomicFile.getResult();
}
}

if(static_cast<WriteASCIIDataFilter::OutputStyle>(fileType) == WriteASCIIDataFilter::OutputStyle::MultipleFiles)
Expand All @@ -211,9 +221,10 @@ Result<> WriteASCIIDataFilter::executeImpl(DataStructure& dataStructure, const A
return MakeErrorResult(-11022, fmt::format("Unable to create output directory {}", directoryPath.string()));
}
}
OStreamUtilities::PrintDataSetsToMultipleFiles(selectedDataArrayPaths, dataStructure, directoryPath.string(), messageHandler, shouldCancel, fileExtension, false, delimiter, includeIndex,
includeHeaders, maxValPerLine);
return OStreamUtilities::PrintDataSetsToMultipleFiles(selectedDataArrayPaths, dataStructure, directoryPath.string(), messageHandler, shouldCancel, fileExtension, false, delimiter, includeIndex,
includeHeaders, maxValPerLine);
}

return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ Result<> WriteAvizoRectilinearCoordinateFilter::executeImpl(DataStructure& dataS
{
AvizoWriterInputValues inputValues;

AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key), true);
AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
auto creationResult = atomicFile.getResult();
if(creationResult.invalid())
{
return creationResult;
}

inputValues.OutputFile = atomicFile.tempFilePath();
inputValues.WriteBinaryFile = filterArgs.value<bool>(k_WriteBinaryFile_Key);
Expand All @@ -106,7 +111,14 @@ Result<> WriteAvizoRectilinearCoordinateFilter::executeImpl(DataStructure& dataS
inputValues.Units = filterArgs.value<StringParameter::ValueType>(k_Units_Key);

auto result = WriteAvizoRectilinearCoordinate(dataStructure, messageHandler, shouldCancel, &inputValues)();
atomicFile.setAutoCommit(result.valid());
if(result.valid())
{
if(!atomicFile.commit())
{
return atomicFile.getResult();
}
}

return result;
}

Expand Down
Loading

0 comments on commit d1b4b50

Please sign in to comment.