TIMOB-13020 First pass attempt to unroll and keep propertyChanged in a single UI callback. #3959
Conversation
…a single UI callback.
|
Maybe I'm missing something here, but from my understanding you're trying to make it so we don't jump from the runtime thread to the main thread for each property in applyProperties()? If so, why don't we just move the current applyProperties() logic into a new method called doApplyProperties(), and just check if it's in the UI thread. If it is, call doApplyProperties() else, send it over to the UI thread and then call doApplyProperties(). |
|
The reason was that I was trying to minimally change behavior as possible. That is, if I were to jump to UI thread in a blocking manner, there'd be a performance hit. Were I to jump in a nonblocking manner, there'd be a race condition as doing foo.applyProperties({bar:5});foo.bar; (or perhaps a more complicated property) may allow for foo.bar to be read before the 5 is applied. I wanted to cause as minimal behavior change (and risk of regression) as possible. I unraveled the old applyProperties and found there were three situations: One in the main thread, one in a background thread, and one where the modellistener was null, which mooted the difference. That's why it has three different codepaths, and only one that does the callout. |
| setProperty(name, value); | ||
| if (shouldFireChange(current, value)) { | ||
| modelListener.propertyChanged(name, current, value, this); | ||
| } |
There was a problem hiding this comment.
Can't we just call setAndFireProperty() instead of doing all this?
There was a problem hiding this comment.
setAndFireProperty has a lot of checks that we can aggregate, such as TiApplication.isUIThread() and checking on modelListener. Part of the optimization was to remove redundant, unnecessary checks.
|
Code reviewed, left comments. |
|
Comments responded. |
|
Code reviewed and functionally tested. Request Accepted. |
TIMOB-13020 First pass attempt to unroll and keep propertyChanged in a single UI callback.
Test is in Jira.