Remove redudant numpy.array()

[InvincibleRMC]

Description

When constructing a msg for a field that is stored in a numpy array it calls the numpy.array() in the constructor and then again inside the property setter. This only seems to be the case for these arrays for whatever reason. For array.array backed field for example it does the conversion within the setter property. For the message I tested where I set all 11 numpy array back fields it resulted in a 15-20% speed improvement on construction.

Methodology

def make_array() -> Arrays:
    return Arrays(
            char_values=[0, 1, 2],
            float32_values=[1.0, 2.0, 3.0],
            float64_values=[4.0, 5.0, 6.0],
            int8_values=[3, 4, 5],
            uint8_values=[6, 7, 8],
            int16_values=[1, 2, 3],
            uint16_values=[1, 2, 3],
            int32_values=[1, 2, 3],
            uint32_values=[1, 2, 3],
            int64_values=[1, 2, 3],
            uint64_values=[1, 2, 3]
        )


def test_timing() -> None:

    out = timeit.repeat(
        make_array,
        repeat=10000000,
        number=1
    )
    med = statistics.median(out)
    mean = statistics.mean(out)
    mode = statistics.mode(out)

    assert f'median: {med}, mean: {mean}, mode: {mode}' == 0

Collected data

100,000 Instantiates repeated 100 times

No duplicate np.array call
median: 1.7657687720002286, mean: 1.7685977168901081, mode: 1.8006701040012558
Duplicated np.array call
median: 2.1092314300003636, mean: 2.110108332149866, mode: 2.1043354339999496

1 Instantiates repeated 1,000,000 times
No duplicate np.array call
median: 1.8749997252598405e-05, mean: 1.866378419397697e-05, mode: 1.8961000023409724e-05
Duplicated np.array call
median: 2.242299888166599e-05, mean: 2.229084920763089e-05, mode: 2.2606000129599124e-05

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

[fujitatomoya]

lgtm with green CI.

[fujitatomoya]

Pulls: #232
Gist: https://gist.githubusercontent.com/fujitatomoya/1430fdb3c98ca41b293e2a4acf064ac1/raw/3e7c93448447610deebc08d2ba0c7cf57e59731a/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_generator_py
TEST args: --packages-above rosidl_generator_py
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17074

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[ahcorde]

should we backport this ? @InvincibleRMC or @fujitatomoya

[fujitatomoya]

@Mergifyio backport kilted jazzy humble

[mergify]

backport kilted jazzy humble

✅ Backports have been created

• #233 Remove redudant numpy.array() (backport #232) has been created for branch kilted but encountered conflicts
• #234 Remove redudant numpy.array() (backport #232) has been created for branch jazzy but encountered conflicts
• #235 Remove redudant numpy.array() (backport #232) has been created for branch humble but encountered conflicts

[fujitatomoya]

i think there is no downside to backport this, @InvincibleRMC correct me if i am wrong on that?

[InvincibleRMC]

i think there is no downside to backport this, @InvincibleRMC correct me if i am wrong on that?

Yeah should be fine.