Skip to content

BUG: CreateStringArrayAction allocates every string during preflight #1892

Open

Description

@joeykleingers

Problem

CreateStringArrayAction ignores the action mode. In preflight it allocates one std::string for every tuple and creates a populated StringArray, instead of a placeholder store as CreateArrayAction and CreateNeighborListAction do. Preflight runs every time a parameter changes in the GUI, so a filter that creates a large string output allocates memory proportional to its tuple count on each preflight. An empty std::string takes about 24 bytes, so a 100 million tuple string array costs about 2.4 GB per preflight.

Expected: in IDataAction::Mode::Preflight the action creates the StringArray with an EmptyStringStore, which holds only the tuple shape. Actual: the array holds totalTuples real strings.

Seen on develop at 705a1b7. 22 filters use the action, for example Read CSV File, Read String Data Array, Concatenate Data Arrays and the EBSD readers.

Steps to Reproduce

  1. Configure simplnx with SIMPLNX_BUILD_TESTS=ON at 705a1b7.
  2. Add the test below to test/EmptyStringStoreTest.cpp.
  3. Build simplnx_test and run ctest --test-dir <build-dir> -R "CreateStringArrayAction" -j 1 --output-on-failure.
  4. The test fails: the array created in preflight is not a placeholder.

Cause

CreateStringArrayAction::apply() receives mode but never reads it. It always builds the full value vector and a populated array (CreateStringArrayAction.cpp#L48-L51):

  usize totalTuples = std::accumulate(m_Dims.cbegin(), m_Dims.cend(), static_cast<usize>(1), std::multiplies<>());

  std::vector<std::string> values(totalTuples, m_InitializeValue);
  StringArray* array = StringArray::CreateWithValues(dataStructure, name, m_Dims, values, dataObjectId);

The numeric path chooses a placeholder in preflight (ArrayCreationUtilities.hpp#L188), and pipeline preflight applies every action in that mode (PipelineFilter.cpp#L196).

Verification Unit Test

File: test/EmptyStringStoreTest.cpp. It also needs these includes:

#include "simplnx/DataStructure/DataStructure.hpp"
#include "simplnx/DataStructure/StringArray.hpp"
#include "simplnx/Filter/Actions/CreateStringArrayAction.hpp"
#include "simplnx/UnitTest/UnitTestCommon.hpp"
TEST_CASE("CreateStringArrayAction: preflight creates a placeholder store")
{
  DataStructure dataStructure;
  const DataPath arrayPath({"Strings"});
  const CreateStringArrayAction action(ShapeType{1000}, arrayPath, "initial");

  Result<> result = action.apply(dataStructure, IDataAction::Mode::Preflight);
  SIMPLNX_RESULT_REQUIRE_VALID(result);

  const auto* stringArray = dataStructure.getDataAs<StringArray>(arrayPath);
  REQUIRE(stringArray != nullptr);
  REQUIRE(stringArray->getNumberOfTuples() == 1000);
  REQUIRE(stringArray->isPlaceholder());
}

Run only this test:

ctest --test-dir <build-dir> -R "CreateStringArrayAction" -j 1 --output-on-failure --timeout 1500 --no-tests=error

Failure seen on macOS arm64, Release build at 705a1b7:

test/EmptyStringStoreTest.cpp:78: FAILED:
  REQUIRE( stringArray->isPlaceholder() )
with expansion:
  false
test cases: 1 | 1 failed
assertions: 4 | 3 passed | 1 failed

Proposed Fix

In CreateStringArrayAction::apply(), branch on mode. For Mode::Preflight, create the array without values and give it an EmptyStringStore, as the metadata-only .dream3d import already does in Dream3dIO.cpp:

  if(mode == Mode::Preflight)
  {
    StringArray* array = StringArray::CreateWithValues(dataStructure, name, m_Dims, {}, dataObjectId);
    // ... same nullptr error handling as today
    array->setStore(std::make_shared<EmptyStringStore>(m_Dims));
    return {};
  }

Keep the current code for Mode::Execute.

An EmptyStringStore throws on element access. Before the change, check that no preflightImpl() and no code that runs during pipeline preflight reads the values of a string array that an earlier filter created through this action, because those reads would start to throw. Add an execute-mode test that checks the array holds totalTuples copies of the initial value.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions