alecjacobson / gptoolbox

Matlab toolbox for Geometry Processing.
MIT License
628 stars 166 forks source link

Removed issue with bsxfun doing full multiplication #117

Closed Kevin-Mattheus-Moerman closed 2 years ago

Kevin-Mattheus-Moerman commented 3 years ago

This pull request "fixes" the poor performance of the loop function. For large meshes the old code becomes very slow.

See here a simple test for icosahedron refinement: slow

The issue was with this line:

Seven(interior,:) = ...
        bsxfun(@times,A(interior,:),beta(interior)) + ...
        sparse(1:ni,original(interior),1-val(interior).*beta(interior),ni,no);

In particular this part:

bsxfun(@times,A(interior,:),beta(interior))

The variable A is a sparse array. However beta is not. Hence if interior features n elements then beta(interior) is a full nx1 array despite the fact that A(interior,:) is a sparse array. I am not sure if the inefficiency of the multiplication comes from the repeated use of the full column or if it temporarily creates a large full array that matches the size of A(interior,:). Either way the following code removes the inefficiency and processing the multiplication only of the non-zero entries in A(interior,:):

A_interior=A(interior,:); %Get only interior part of A
    [rowIndices,~,~]=find(A_interior>0); %Get column insices for all non-zero entries
    A_beta_interior=A_interior; %Initialize array for multiplication of A with beta
    A_beta_interior(A_interior>0)=A_interior(A_interior>0).*beta(interior(rowIndices)); %Process multiplication only for non-zero entries
    Seven(interior,:) = A_beta_interior + sparse(1:ni,original(interior),1-val(interior).*beta(interior),ni,no);

Following this change the same performance graph becomes: faster

MATLAB version notes:

MATLAB Version: 9.10.0.1684407 (R2021a) Update 3
Operating System: Linux 5.4.0-77-generic #86-Ubuntu SMP Thu Jun 17 02:35:03 UTC 2021 x86_64
Java Version: Java 1.8.0_202-b08 with Oracle Corporation Java HotSpot(TM) 64-Bit Server VM mixed mode

Computer notes:

CPU: Intel® Core™ i7-4910MQ CPU @ 2.90GHz × 8
RAM: 32Gb
alecjacobson commented 2 years ago

Wow. So it seems like:

bsxfun(@times,A(interior,:),beta(interior))

is way slower (dense?) than the more modern, broadcasting .*

A(interior,:).*beta(interior)

I pushed this fix in: https://github.com/alecjacobson/gptoolbox/commit/7d3e5563da7d361b55f2dcfe5b43d9e0e61be843

I believe that fixes the same issue in this PR. I'm inclined to close this and use the simpler .* change. Thanks for pointing out this issue!