Skip to content

Commit

Permalink
fix for deadlock (due to ill-partioned memory) (#5)
Browse files Browse the repository at this point in the history
  • Loading branch information
ptahmose authored Jun 11, 2024
1 parent 72148b3 commit 31b2f65
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
set (CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/modules" ${CMAKE_MODULE_PATH})

project ("warpaffine"
VERSION 0.3.1
VERSION 0.3.2
DESCRIPTION "experimental Deskew operation")

option(WARPAFFINE_BUILD_CLANGTIDY "Build with Clang-Tidy" OFF)
Expand Down
28 changes: 25 additions & 3 deletions libwarpaffine/configure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ bool Configure::DoConfiguration(const DeskewDocumentInfo& deskew_document_info,
// * We limit the memory available for "destination brick" to about 1/3 of the main-memory
// * We set the high-water mark to 60% of the main memory. The high-water mark determines around when the
// reader throttles, or - when that amount of memory is allocated (in total), then the reader throttles.

uint64_t high_water_mark_limit = this->physical_memory_size_ * 6 / 10;

// high_water_mark_limit must be larger than "memory_characteristics.max_size_of_input_brick" - if this is not the case
Expand All @@ -73,12 +73,34 @@ bool Configure::DoConfiguration(const DeskewDocumentInfo& deskew_document_info,

uint64_t limit_for_memory_type_destination_brick = this->physical_memory_size_ / 3;

// the limit for the memory type "destination brick" must not lower than the "max_size_of_output_brick_including_tiling"
// (in fact, it must also not be equal to it)
if (limit_for_memory_type_destination_brick <= memory_characteristics.max_size_of_output_brick_including_tiling)
{
limit_for_memory_type_destination_brick = memory_characteristics.max_size_of_output_brick_including_tiling + 1;

// we adjust the high_water_mark_limit then...
high_water_mark_limit = (this->physical_memory_size_ - limit_for_memory_type_destination_brick) * 94 / 100;
}

// well, this limit must not be larger than "what's remaining if we subtract the high_water_mark_limit"
if (limit_for_memory_type_destination_brick > this->physical_memory_size_- high_water_mark_limit)
if (limit_for_memory_type_destination_brick > this->physical_memory_size_ - high_water_mark_limit)
{
limit_for_memory_type_destination_brick = this->physical_memory_size_ - high_water_mark_limit;
}

// if the requirements are not fulfilled now, we get out of here - otherwise we likely would deadlock
if (limit_for_memory_type_destination_brick <= memory_characteristics.max_size_of_output_brick_including_tiling ||
high_water_mark_limit <= memory_characteristics.max_size_of_input_brick)
{
ostringstream string_stream;
string_stream.imbue(this->app_context_.GetFormattingLocale());
string_stream << "Unable to process this document: No suitable memory configuration could be determined." << endl
<< "This program will exit now. (Check the synopsis for how the memory-size assumed can be adjusted)" << endl;
this->app_context_.GetLog()->WriteLineStdOut(string_stream.str());
return false;
}

this->app_context_.GetAllocator().SetMaximumMemoryLimitForMemoryType(BrickAllocator::MemoryType::DestinationBrick, limit_for_memory_type_destination_brick);
this->app_context_.GetAllocator().SetHighWatermark(high_water_mark_limit);

Expand Down Expand Up @@ -118,7 +140,7 @@ bool Configure::DoConfiguration(const DeskewDocumentInfo& deskew_document_info,
deskew_document_info.map_brickid_position.cend(),
[](const auto& x, const auto& y)
{
return x.second.width * x.second.height < y.second.width* y.second.height;
return x.second.width * x.second.height < y.second.width * y.second.height;
});

// and now, we determine the "largest pixeltype"
Expand Down
4 changes: 2 additions & 2 deletions libwarpaffine/warpaffine/WarpAffine_IPP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ void WarpAffineIPP::ExecuteMinimalSource(

// As of the time of writing, the IPP has trouble if the source or the destination is
// beyond 2GB it seems. What we do here - if the source or the destination buffer is
// larger than 2GB, we fall back to the reference implemenation.
// larger than 2GB, we fall back to the reference implementation.
constexpr uint64_t kSafeSizeForIppOperation = numeric_limits<int32_t>::max();
if (static_cast<uint64_t>(source_brick.info.stride_plane) * integer_source_voi_clipped.depth > kSafeSizeForIppOperation ||
static_cast<uint64_t>(destination_brick.info.stride_plane) * destination_brick.info.depth > kSafeSizeForIppOperation)
{
// The reference-implemenation currently only supports NN and linear interpolation. What we do here now is
// The reference-implementation currently only supports NN and linear interpolation. What we do here now is
// that we "silently" map all supported interpolation modes to this.
// TODO(JBL): this is of course lame
Interpolation interpolation_mode_adjusted;
Expand Down

0 comments on commit 31b2f65

Please sign in to comment.