LabVIEW

cancel
Showing results for 
Search instead for 
Did you mean: 

Converting maps to array slow?

The new version is attached.

 

Changes:

  • the first time is dropped. We know that part is fast, no need to measure
  • if the collection size grew with the last entry (new path) then I increase the array size by 200. I do it only once per new file instead of doing that in every iteration
  • (The VI probably looks even worse aesthetically)

 

Execution time decreased below 7.000ms on my PC which is a great improvement from ~26.000. 

 

I'll incorporate these changes to my original code and check the performance. Thanks for all the inputs so far. (still wondering what could be an alternative solution to the problem)

0 Kudos
Message 11 of 15
(591 Views)

@1984 wrote:

The new version is attached.


This simply does not smell right, IMHO.

 

  • What are the defaults for all controls and how is it called? Shouldn't in init when i=0 for both loops? Currently it "inits" always or never.
  • Why is the map key a string instead of a path?
  • Why do you call an indicator a constant ("map constant")?
  • Under what conditions is the second case structure ever FALSE? (never, because the path is never empty!)
  • Why do you only time a small subset of the code? That part is connected to a last value tunnel, and could be done once after the FOR loop, right?
  • Why is the collection size diagram constant U32?
  • If your array size is exactly 200, you are replacing an index (200) that does not exist.
  • Your random generator is not fair. Edge values only have half the probability of most.

 

 

So please clarify in a few more words what you are trying to do. You have a max of 11 map entries that are consecutive and sorted. Is that typical? Maybe you don't even need a map!

 

 

0 Kudos
Message 12 of 15
(573 Views)


This simply does not smell right, IMHO.

 

  • What are the defaults for all controls and how is it called? Shouldn't in init when i=0 for both loops? Currently it "inits" always or never.
  • Why is the map key a string instead of a path?
  • Why do you call an indicator a constant ("map constant")?
  • Under what conditions is the second case structure ever FALSE? (never, because the path is never empty!)
  • Why do you only time a small subset of the code? That part is connected to a last value tunnel, and could be done once after the FOR loop, right?
  • Why is the collection size diagram constant U32?
  • If your array size is exactly 200, you are replacing an index (200) that does not exist.
  • Your random generator is not fair. Edge values only have half the probability of most.

 

 

So please clarify in a few more words what you are trying to do. You have a max of 11 map entries that are consecutive and sorted. Is that typical? Maybe you don't even need a map!

 


I dont see how any of these questions are relevant. I mean for example what does a badly named indicator or an unfair path generator do the execution speed? I have stated multiple times that the VI I uploaded is a significantly altered version of the original and used only for measuring performance and experimenting. The VI I use in my app runs once at a time. The for loop is only there so I dont need to push the run button hundreds of times.

 

I think having ~10 paths is quite typical scenario, but other than this it was an arbitrary number just to see whats the performance of this VI if the map size grows.

 

I also wrote multiple times that I went for maps because at that time I thought its a good idea (and I still dont think that it was a bad idea), but I'm looking forward to see how others would solve the same problem. So if you have a different solution in mind then dont hesitate sharing it cause it would be highly appreciated.

0 Kudos
Message 13 of 15
(528 Views)

@1984 wrote:
I dont see how any of these questions are relevant. d.

No need to be offended, but in my experience, the general "code smell" typically indicates that there are other serious problems.

 

A map is most useful if the number of entries is large (insert, lookup, delete operation are O((logN) instead of O(N)), so for N=10 the difference is irrelevant, You could do a fully in-place data structure instead. I agree that the map operations are not the limiting step.
It is all in the part that you are timing, i.e. rearranging all data into a new structure. This needs to be done only once you need that output, and not 10k times for nothing. It does not seem reasonable to do efficient map operations, then spend gigantic efforts to immediately rearrange all data into a new structures with every iteration of the loop. That's why I asked how this is actually used. I understand that your real application is more complex but it seems pointless to do that processing unless you need that output right there and then.

 

If we clean things up a bit and take that final data rearrangement out of the loop, things are quite fast (and this is on my very, very old laptop!)

It also seems redundant to have the key be part of the map data.

 

altenbach_0-1684074286289.png

 

 

Message 14 of 15
(475 Views)

I didnt get offended I just didnt understand what those questions do with the problem itself. The code in my application doesnt smell that way. Besides this your response was very useful (as well the others) so we have decided to change the behaviour of the VI. We most likely will be able to get rid of the map-> cluster conversion but even if we cant we can move this part of the code to an async VI.

 

Thanks for all the inputs.

 

 

Message 15 of 15
(399 Views)