Could ITK drop ITK_USE_64BITS_IDS and just do SizeValueType = std::size_t?

Engineering
1

It appears quite hard to fully support 64-bit memory address space (including images of more than 4 GB), in ITK source code. On many places, it appears that a 64-bit “size” or offset value is converted to 32-bit (depending on platform, compiler and CMake options), which could cause a crash when an ITK based user application processes large image data.

I did a first pull request, which fixes some cases of narrowing conversion from 64-bit “size type” to 32-bit: Pull request #1087: Fix possible loss of data on type conversion But honestly, this is just a tip of the iceberg!

A thing dat makes 64-bit support harder is that C++ and ITK both define various different “size types” and “offset types”, which may or may not have the same number of bits.

It seems much easier to me if ITK would drop the ITK_USE_64BITS_IDS option, and just do

using SizeValueType = std::size_t;

In practice, std::size_t is always 32-bit on a 32-bit application, and 64-bit on a 64-bit application, right? It seems to me that that would be fine for itk::SizeValueType as well. What do you think?

3 Likes
2

This is very much overdue. I support this change, and I will help you do it Niels (assuming consensus is in favor).

3

First, I have had great success using ITK with very large microscopy images, and it has been a long time since I encountered type shortening causing a problem. ITK currently support large (>4GB) images very well IMHO.

The name of ITK_USE_64BITS_IDS is not too accurate as it only applies when “unsigned long” is not of sufficient length for the architecture or explicitly enabled. There are two use case:

  • On win64 for compatibility where unsigned long historically was used, this option could be OFF.
  • On 32-bit architectures when you needed to stream large images, I had some success with this.
    I am either of these options are very important/relevant anymore. But when you get into more obscure configurations such as for ARM or JSON these types may be more interested.

Do you have a suggestion for the related IdentifierType, IndexValueType and OffsetType integer types?

We are now(corrected) in ITK 5.0 maintenance mode, and changes need to be made without breaking code. To move forward I would suggest adding a CMake option ITK_USE_SIZE_T_IDS. We can experiment with this new option and ensure every thing works well. then deprecate and warn about the others when it’s stable. Then in 6.0 it would be the only implementation.

Two notes:

  • even when long and long long are the same size, they are considered different types and interesting error have been know to occur.
  • There are a few certain types that are still unsigned int in ITK. The biggest one in the number of dimensions for an image, and similarly the number of components for pixel types.

Edited: not->now

2 Likes
4

size_t is only 32 bits on ARM, and we want to continue to support processing large images on that platform.

5

Interesting… So are you able to address data larger than 4 GB from memory on that platform, even while its size_t is only 32 bits?

6

Generally, the use cause is stream processing – all the data is not loaded into memory.

Then again, most of the newer and high performance systems are 64-bit ARM (including my phone and the most recent editions of the Raspberry Pi), so size_t could be fine in practice.

7

Matt, the link you provided points to a document from 2011. ARM now has 64-bit processor designs too. And this porting guide is pretty clear that 64-bit ARMs have 64-bit size_t too.

8

Thanks for your extensive reply, Bradley.

I’m fine with the current IdentifierType = SizeValueType. (Which would then be the same as std::size_t.)

For the two signed ITK integer types, I would suggest std::ptrdiff_t:

using IndexValueType = std::ptrdiff_t;
using OffsetValueType = std::ptrdiff_t;

OK?

1 Like
9

After more than 7 years, maybe it is time to reconsider whether ITK_USE_64BITS_IDS is still needed, nowadays… Hans (@hjmjohnson) has just opened a GitHub issue on this topic. Please check:

1 Like