Significant performance degradation on complex model

[fsudaman]

FloorSpace's performance and responsiveness degrades significantly as models get more complex, to the point where it can be quite arduous to work with or almost unusable.

The attached model allow you to reproduce the issue where an average of 1-2 second delay is observed on every mouse click, which makes it frustrating to do anything other than minor adjustments.

Performance Degredation.zip

[Mathadon]

@fsudaman I'm interested to start using floorspace. At what number of zones/rooms/floors/whatever does this start to occur? Thank you!

[Mkellyeng]

Hi @Mathadon, I work with @fsudaman
Give it a go, floorspace is very powerful, it's by far the fastest way to draw geometries that I've found.

The lag accumulates over time, but is not too bad up to 100 or so zones. However this was a 4 storey hospital, so it got complex pretty fast. A workaround that I found was saving and then deleting each floor as I went, to reduce the size and thus the lag. I then stitched the floors together using an online JSON editor. It got the job done, but it was pretty painful, and floorspace would be much better if the lag issue was resolved.

[macumber]

Thanks for the feedback @Mkellyeng, NREL doesn't have any active development activities for FloorspaceJS (we will look at fixing bugs in the OpenStudio FloorspaceJS translator). However, @bgschiller and others at Devetry are very capable of fixing bugs/performance issues in FloorspaceJS. You might reach out to them if you are interested in discussing ways to work on your concerns.

[bgschiller]

@Mkellyeng @fsudaman: As @macumber mentioned, there's not a lot of active development on the project (NREL hired us at Devetry to get it to the current state, but there's no current contract).

That said, I have some thoughts on how to clear up some of the lag. I've been noodling on it since this issue was raised and I think there are two main sources:

1. there are several O(n) operations that could easily be O(1). The main reason for this is that stories, geometries, and other smaller collections are stored as lists. That means whenever we access this, there's code like _.find(state.geometry, { id: geometry_id }). It would likely be faster to change to storing these collections as a lookup table indexed by id, which means we could replace code like that with state.geometry[geometry_id].

Some things to consider: we would need to convert back to a list upon "export", as that's the format expected by OpenStudio downstream. Luckily, we have tests to prevent us messing this up 😁 . Also, it's likely we would need to begin including an explicit "order" property, since objects are not ordered and we're currently relying on the list to maintain the proper order of our stories.

If someone wanted to tackle this, the best place to start would be in the src/modules/geometry. directory.

2. The undo/redo functionality is pretty aggressive—I think it stores an isolated copy of the state after each action. I'm not sure if this directly contributes to slowness, but it certainly uses a lot of memory. Two possible approaches:

   a. Don't allow indefinite undos. Limit the number of past states we store, either by total number or by timestamp.
   b. Remove the JSON.parse(JSON.stringify(state)) business. By avoiding the deep clone, the state is able to share most of its internal structure on a modification, which should dramatically cut memory usage.

To tackle this, you'd want to visit src/store/timetravel.js, where the undo/redo functionality is defined. If you want to test whether undo/redo is truly the source of the slowness, you can easily disable it by commenting out this line in main.js:

timetravel.init(store);

Happy to review and provide feedback if you get a chance to work on this!

[fsudaman]

Thanks @macumber and @bgschiller for the details provided here as well as offering the support. I will discuss this with the team and assess if we can take this on. In parallel, I will drop you an email as well to explore the alternate option.

[aaroncoderepo]

I have noticed that adding a new 'Space' to the model linked by @fsudaman is very time consuming. After adding additional console logging it was discovered that the dropDaylightingControls method in mutations.js is taking over 7 seconds to run.

dropDaylightingControls(state, { story_id }) { const story = _.find(state.stories, { id: story_id }); console.log('number of spaces: ' + story.spaces.length); const timer = new Date(); story.spaces.forEach((space) => { space.daylighting_controls = []; }); console.log('dropDaylightingControls: ', new Date() - timer); },

dispatching action: models/destroyAllComponents
dropDaylightingControls: 7455

The bulk of the time is consumed by the following operation:
space.daylighting_controls = [];

Would it be possible to modify the code by adding an additional 'if' check? This reduces the execution time of dropDaylightingControls to a fraction of a second in our scenario:

dropDaylightingControls(state, { story_id }) { const story = _.find(state.stories, { id: story_id }); story.spaces.forEach((space) => { if(space.daylighting_controls.length > 0) { space.daylighting_controls = []; } }); },

I am not sure of the impact this would have on floorspace? Speeding up this method is not a complete fix, as the operation of adding a Space still takes an additional few seconds.

Some of the suggested items were investigated to see if they had performance improvements. We (@fsudaman, @Mkellyeng and myself) did not find that these changes improved performance:

• removing JSON.Parser
• disabling timetravel.init(store);
• changing the lookup to use native js functions instead of loadash (Ex find vs _.find)
  (lodash find took 178ms to perform the following operation: geom = geometryHelpers.denormalize(_.find(state.geometry, { id: geometry_id })) - while this is significant it did not seem to be the root cause)

Some screenshots:

[bgschiller]

@aaroncoderepo Thanks for your exploration of the causes of the slowness! It will be no problem to add that if-statement.

I'm surprised that the undo/redo stack doesn't seem have an affect on the speed, but 🤷‍♂️.

[aaroncoderepo]

Thanks for reviewing my code, how would you like to proceed @bgschiller ?

I can create a branch, and pull request for this change - but I don't seem to have permissions (403 error) to push a new branch to this repository.

[bgschiller]

@aaroncoderepo you should be able to open a pull request from your fork. If that doesn't work, just point me to where your repo lives.

[aaroncoderepo]

Unfortunately we found that the performance savings we noticed (with the above change) when debugging locally did not translate when deployed. The seven second delay does not appear in our hosted environment (I can only speculate as to why this is the case).

[bgschiller]

Still to do: profile and understand the biggest contributors to slowdown

[Mkellyeng]

Some details to add to this that I've noticed over time

• Images seem to contribute a lot to the slowdown, and deleting images can alleviate some of the slowdown. Pretty noticeable on multi-story, moderately complex buildings. Images also seem to increase file size a lot
• Slowdown seems to accumulate over time, sometimes saving and refreshing seems to reduce slowdown a little (only noticeable on bigger models made during a single session). Anecdotal only. This will sometimes culminate in floorspace crashing

The libraryObjectWithId function is a possible culprit (helpers.js). It seems to take an ID and tries to figure out what it belongs to by looking everywhere in the file for it, but the way it searches seems pretty inefficient. It looks like it gets called frequently

[benfen]

Looking at the performance chart in chrome, the browser spends an awful lot of time running through traverse functions when it updates the dimensions. That seems to be a Vue internal, and is possibly related to data structure, as @bgschiller mentioned a while back. I need to dig into that a little more, but that seems to take up a large percentage of the time (~70-80%) when the browser is running through things.

The other thing that jumps out to me is that dimensions are frequently set one by one, e.g.

floorspace.js/src/components/Grid/methods.js

Line 1338 in fe341ea

[this.min_x, this.max_x] = newScaleX.domain();

. This means that it runs through the change detection process four times, instead of once, which may be exacerbated by the aforementioned issue. Doing some quick testing within the zoom function, that does seem to help a bit, but it's not a silver bullet.

@Mkellyeng Do you have an example with some images to do testing with?

[benfen]

I put together a PR with some of the changes - #392

The changes are a little rough around the edges.

[macumber]

@benfen do the performance updates in #392 address speed of non-drawing tasks as referenced in openstudiocoalition/OpenStudioApplication#118 ?

[benfen]

In the webapp, those actions are all quite quick. However, they were fast before any of these performance changes were done, so it was likely fixed by another change.

[benfen]

I took a second look at it and I'm seeing some of those same issues; I'll look into speeding them up

[benfen]

@macumber #400 addresses the issue of those non-drawing tasks. The performance issue there seemed to come from the element-ui library; removing it and adding in a library for virtual scrolling reduces the load time immensely.

[macumber]

Awesome!