Performance issue with the redundancy operations

[yyzdtccjdtc]

visioncortex/src/color_clusters/builder.rs

Line 401 in 402bd07

let view = self.view();

This view() function will create a ClustersView structure inside a loop, but for the rest code of this loop, only the width, height and clusters_indices will be used. And for every loop iteration, this view() function will create the same values again and again which introduce a lot of redundancy operations. Perhaps we can dispense with this view() function and instead directly reference the self first and then get the needed variables inside these functions.

If we change the code of

visioncortex/src/color_clusters/builder.rs

Line 416 in 402bd07

.neighbours(&view)

Into

.neighbours(&self)

(Need to do some modifications with the neighbours() function)

Change

visioncortex/src/color_clusters/builder.rs

Lines 438 to 439 in 402bd07

	let deepen = if self.hierarchical == HIERARCHICAL_MAX {
	(self.deepen)(&view, &self.get_cluster(index), &infos)

Into

            let deepen = if self.hierarchical == HIERARCHICAL_MAX {
                (self.deepen)(&self, &self.get_cluster(index), &infos)

(Need to modified the deepen function too.)

Change

visioncortex/src/color_clusters/builder.rs

Line 443 in 402bd07

let hollow = (self.hollow)(&view, &self.get_cluster(index), &infos);

Into

let hollow = (self.hollow)(&infos);

Then comment out the line 401 and 436.

visioncortex/src/color_clusters/builder.rs

Line 401 in 402bd07

let view = self.view();

visioncortex/src/color_clusters/builder.rs

Line 436 in 402bd07

let view = self.view();

Then it can avoid these redundant operations.

According to my test it will get a 43.8% speedup (7.3s to 5.1s) with the vtracer application (https://github.com/visioncortex/vtracer).

Hope this information helps!

[tyt2y3]

That's outrageous number! Just a sanity test, did you benchmark it with --release?
It's a bad coding pattern actually. I think we should copy all methods to the Cluster itself so that view() is no longer needed.
On the other hand, what's preventing LLVM from optimizing things? Can we add an #[inline] to view() and see if that does the trick?

[tyt2y3]

Anyway, welcome!

[yyzdtccjdtc]

@tyt2y3
Hi Chris,

Yes, I am testing it with the --release and set opt-level = 3 in the Cargo.toml file. According to the output file from the objdump, this is not optimized because this view() function stores these values from the registers on the stack and then takes them out of the stack when they are needed. So I believe that this inline keyword does not change anything, and my test also shows that the running time did not change after adding this inline keyword for view() function.

[tyt2y3]

Sad. Thank you for the investigation. PR would be welcomed. Though I would prefer keeping the view() function in place because other crates depended on it.

[yyzdtccjdtc]

Hi, I submitted a pull request. Here's the link #5 (comment)