Skip to content

3 optimizations (especially eliminating double-calculation of traffic distance between traffic cache and audible alerts)#4

Merged
apps4av merged 8 commits into
apps4av:masterfrom
shanelenagh:PR_distance_opt
Mar 31, 2024
Merged

3 optimizations (especially eliminating double-calculation of traffic distance between traffic cache and audible alerts)#4
apps4av merged 8 commits into
apps4av:masterfrom
shanelenagh:PR_distance_opt

Conversation

@shanelenagh
Copy link
Copy Markdown
Contributor

Doing some inspection and CPU flame chart analysis of AvareX while traffic was on, I found 3 opportunities for optimization, which are addressed with this PR (the third of which doesn't deal with traffic per se, but it was something I noticed in the chart while looking at the others):

  1. Traffic distance was being computed twice, once by the traffic cache in the sort method for limiting the display (and consequently audible alerts) to the "top 20" traffic items, and then again by the audible alerts itself. I instead changed the traffic cache to do the computation once on each traffic instantiation (and then again for all traffic in the cache when ownship changes position, which would affect the prior computation for all traffic, of course), and persist that on fields on the traffic object. That is then used by the audible alerts, which no longer needs to do it itself. Also, as part of this, I made the algorithm for the computation use the Haversine formula (what AA was using), which is within 0.3% (3/10 of a percent) of the Vicenty formula for accuracy, but is much, much faster (CPU flame chart analysis bore this out). And I am using the same library, LatLong, for that, as they allow you to select either Vicenty or Haversine in the constructor. I think Vicenty makes sense in the rest of the app where high precision for less frequent calculations might be a higher concern, but when it is computing it for every traffic item often many times per second (per ADSB updates) Haversine makes more sense.
  2. Changed audible alerts calls to not step on themselves. Both the flame chart and some debugging showed the AA feature being called ridiculously frequently (every single traffic item insert) and processing every traffic item again when there were no changes. Yes, I had code in there to exit when there was no updated timestamp, but even the multiple iterations through the cache and the hashmap check were showing up in the chart. Now it calls the AA processing loop no more frequently than once every 100 milliseconds, which is plenty frequently still.
  3. There was a dateformat that was showing up on the flame chart in the Instruments code. I had just watched a video from the Google Flutter team on performance optimization where they used this exact thing as an example (https://youtu.be/vVg9It7cOfY?si=WcUmLXwQc66nQOM-&t=2386), so I knew the simple solution--make that format (which has no per-instance unique properties) a static of the class. That removed it from the prominent space it was taking on the chart.

I can provide some screenshots (say of the before/after flamecharts) and screencast demo's if required; let me know. They did have a demonstrable impact when I ran them on my device (my screaming G16 laptop with the RTX 3070 Ti didn't care, of course, but that isn't what I will be flying with :-)

… (made static final), based on CPU flamechart results
…g LatLong library) for doing fast distance calculation, when speed is more important than (still imperfect) accuracy
…y once--in traffic cache, using fast Haversine calculation (provided by existing LatLong library)
…ls, rather scheduling on a min delay interval, to prevent useless thrashing of the alert checker loop
Comment thread lib/gdl90/traffic_cache.dart Outdated
…string interpolation performance hit shown in flame chart
…tring interpolation hit showing up in the flamechart
…ust--or as robust as it can be without language synchronization primitives (e.g., monitors)
@shanelenagh
Copy link
Copy Markdown
Contributor Author

Zubair, I was wrong, there were no merge conflicts even after you merged in the other PR--Git never ceases to amaze me with how well is does these things! So you can merge this whenever you feel comfortable, @apps4av, as there is no need for me to rebase and resolve conflicts (there are none, from what I see and what Github is saying here).

@apps4av apps4av merged commit 3a21f0b into apps4av:master Mar 31, 2024
apps4av added a commit that referenced this pull request May 26, 2026
Fixes the security audit findings on the v1 Community rules. All
changes are rules-only except a small members-screen tweak so that
non-members of private groups see a friendly message instead of a raw
permission-denied error.

Critical / high:
- Counter updates now require an exact +/-1 delta and forbid negative
  values, instead of allowing any signed-in user to set memberCount
  and postCount to arbitrary numbers (audit #1). Long-term fix is a
  Cloud Function trigger; documented in the rules file.
- All user-writable documents (profile, group, member, post,
  userGroups index) are now whitelisted on field keys and validated
  on types, lengths, and enum values, closing latent privilege
  escalation paths and DoS via oversized documents (audit #2, #4, #8,
  #12, #13).
- Removed the unconstrained isGroupOwner(gid) branch on member create.
  Owner approvals now flow through update of an existing pending doc,
  with status pinned to pending -> active and role/joinedAt frozen
  (audit #3).

Medium:
- Member rosters are no longer enumerable by strangers. Reads of
  groups/{gid}/members require self, owner, public visibility, or
  active membership (audit #6).
- Posts are now bounded to 1000 characters at the rule level (audit
  #7) and to a fixed field whitelist (audit #8).

Hardening:
- isActiveMember consolidated to a single get() lookup pattern via
  helper functions (audit #10 partial).
- All timestamps validated as `is timestamp`; full server-timestamp
  enforcement deferred to a future Cloud Function (audit #13).

Knowingly not fixed in v1 (documented inline):
- Group metadata visibility: the group doc remains readable by any
  signed-in pilot so the Discover tab can surface private groups by
  name. Sensitive future fields must live in a separate restricted
  subcollection (audit #5).
- Counter drift under adversarial use: ruled-bounded to +/-1 per call;
  long-term fix requires Cloud Functions (audit #11).
- Owners can technically self-delete their membership through this
  rule; rules cannot distinguish "owner leaves" from "owner deletes
  group". The client repository blocks owner-leave at call time
  (audit #9).

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants