Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple fixes to improve performance of the new label screen #902

Merged
merged 4 commits into from
Oct 29, 2022

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Oct 26, 2022

This is a snapshot before we start the restructuring with web workers.

- Convert the mapLimiter from a potentially modifiable `$scope` variable to a
  const, so angular doesn't need to watch it
- Change the schedule so that we pass in the promise to schedule, and handle
  the callback from the schedule callback.

This ensures that we launch the location data code after the list has been displayed

```
[Log] DEBUG:After adding batch of size 39 cumulative size = 39 (index.html, line 154)
[Log] DEBUG:new end time = 1665705805.000558 (index.html, line 154)
[Log] true (index.html, line 154, x39)
[Log] DEBUG:Broadcasting infinite scroll complete (index.html, line 154)
[Log] DEBUG:infiniteScrollComplete broadcast (index.html, line 154)
...
[Log] DEBUG:About to pull location data for range Thu Oct 20 2022 15:51:18 GMT-0700 -> Thu Oct 20 2022 16:14:32 GMT-0700
[Log] DEBUG:About to pull location data for range Thu Oct 20 2022 10:20:23 GMT-0700 -> Thu Oct 20 2022 10:22:34 GMT-0700
DEBUG:About to pull location data for range Thu Oct 20 2022 08:21:40 GMT-0700 -> Thu Oct 20 2022 08:34:07 GMT-0700
...
[Log] DEBUG:Retrieved 52 points at Tue Oct 25 2022 22:08:42 GMT-0700 (PDT) (index.html, line 154)
[Log] DEBUG:Retrieved 6 points at Tue Oct 25 2022 22:08:42 GMT-0700 (PDT) (index.html, line 154)
[Log] DEBUG:Retrieved 28 points at Tue Oct 25 2022 22:08:42 GMT-0700 (PDT) (index.html, line 154)
...
DEBUG:About to pull location data for range Wed Oct 19 2022 20:56:45 GMT-0700 -> Wed Oct 19 2022 21:09:41 GMT-0700
DEBUG:About to pull location data for range Wed Oct 19 2022 17:33:27 GMT-0700 -> Wed Oct 19 2022 17:46:04 GMT-0700
DEBUG:About to pull location data for range Wed Oct 19 2022 15:01:10 GMT-0700 -> Wed Oct 19 2022 15:18:25 GMT-0700
```
@shankari
Copy link
Contributor Author

`getDisplayName` is now an abomination unto good programming taste.
It takes two inputs: one which is a "mode" and one which an "obj"

Depending on the mode, it retrieves the appropriate field from the object,
calls nominatim, and then puts the values back into the object.

This is really ugly because:
- we need a switch statement for each object type in this function, thus
  breaking modularity and leading to weird contortions (as in the infinite
  scroll) if we wanted to support a new type of object that read and wrote to
  different places
- we needed two callback functions to process the nominatim result, one to set
  the field for everything other than the ctrip end place and the other for
  just the ctrip end place

With the new code, we simply refactor to a promisifed function that takes a
location object and returns the matching name. We change all existing call
sites to extract the location correctly, and with `then` to set the value back
correctly.

Testing done:

values read properly in infinite scroll

```
[phonegap] [console.log] 22:48:45 GMT-0700 (PDT) Getting display name for  [object Object]
[phonegap] [console.log] 22:48:51 GMT-0700 (PDT)while reading data from nominatim, status = 200 data =
[phonegap] [console.log] 22:48:51 GMT-0700 (PDT)got response, setting display name to ...
```

values read properly in diary

```
[Log] Don't have display name for start place, going to query nominatim
[Log] 22:52:43 GMT-0700 (PDT) Getting display name for  – {type: "Point", coordinates: [-122.08652898438098, 37.39099341901742]}
[Log] Don't have display name for end place, going to query nominatim
[Log] 22:52:43 GMT-0700 (PDT) Getting display name for  – {type: "Point", coordinates: [-122.08532730526939, 37.38950960523626]}
[Log] Don't have display name for start place, going to query nominatim
[Log] 22:52:43 GMT-0700 (PDT) Getting display name for  – {type: "Point", coordinates: [-122.08532730526939, 37.38950960523626]}
[Log] Don't have display name for end place, going to query nominatim
[Log] 22:52:43 GMT-0700 (PDT) Getting display name for  – {type: "Point", coordinates: [-122.08644288085807, 37.39101601640591]}
[Log] 22:52:49 GMT-0700 (PDT)while reading data from nominatim, status = 200 data =
[Log] 22:52:49 GMT-0700 (PDT)got response, setting display name to South Shoreline Boulevard, Mountain View
```
@shankari shankari merged commit 08df383 into e-mission:master Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant