From ac42896271e3ea9b8e047c17ce35359f1ee8ca30 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 19 Mar 2026 12:41:24 -0500 Subject: [PATCH] fix: performance improvements and bug fixes across backend and frontend Backend: - Use indexed has_weather boolean for weather queries instead of 6 OR conditions - Enable gzip compression for static assets - Re-enable security headers (CSP, X-Frame-Options, X-Content-Type-Options) - Supervise cleanup task via BroadcastTaskSupervisor instead of Task.start - Relax IsSupervisor restart limits (5/60s) to survive transient network issues - Remove dead code: empty cache invalidation block, commented-out logging Frontend: - Use callsignIndex for O(1) marker dedup instead of O(n) forEach scan - Consolidate triple packet loop into single pass in new_packets handler - Batch trail visualization updates via requestAnimationFrame - Replace DOM querySelectorAll z-index scan with simple counter - Only rebuild OMS markers when crossing clustering zoom threshold - Clean up trail manager on hook destroy - Remove dead commented-out localStorage code --- assets/js/features/trail_manager.ts | 38 ++++++++++++++-- assets/js/map.ts | 68 ++++++++++++++--------------- assets/js/map_helpers.ts | 9 ---- lib/aprsme/cleanup_scheduler.ex | 3 +- lib/aprsme/is/is_supervisor.ex | 5 +-- lib/aprsme/packet_consumer.ex | 11 ----- lib/aprsme/packets.ex | 38 +++++----------- lib/aprsme_web/endpoint.ex | 2 +- lib/aprsme_web/router.ex | 8 ++-- 9 files changed, 85 insertions(+), 97 deletions(-) diff --git a/assets/js/features/trail_manager.ts b/assets/js/features/trail_manager.ts index a04ee48..10b6dcb 100644 --- a/assets/js/features/trail_manager.ts +++ b/assets/js/features/trail_manager.ts @@ -31,6 +31,8 @@ export class TrailManager { private trailDuration: number; // in milliseconds private maxTrails: number = 500; // Maximum number of trails to keep in memory private maxPositionsPerTrail: number = 1000; // Maximum positions per trail + private pendingTrailUpdates: Map = new Map(); // callsign -> isHistorical + private trailUpdateRafId: number | null = null; // Colors ordered to maximize contrast between consecutive assignments. // Avoids yellows, oranges, and grays that blend with roads on map tiles. private colorPalette: string[] = [ @@ -207,9 +209,31 @@ export class TrailManager { }); } - // Always update trail visualization, with immediate=true for historical dots - // This ensures historical trails are immediately visible when loaded all at once - this.updateTrailVisualization(baseCallsign, trailState, isHistoricalDot); + // Defer trail visualization to batch multiple updates into one frame + this.scheduleTrailUpdate(baseCallsign, isHistoricalDot); + } + + private scheduleTrailUpdate(baseCallsign: string, isHistorical: boolean) { + // Track which trails need updating; preserve isHistorical=true if set + const existing = this.pendingTrailUpdates.get(baseCallsign); + this.pendingTrailUpdates.set(baseCallsign, existing || isHistorical); + + if (this.trailUpdateRafId === null) { + this.trailUpdateRafId = requestAnimationFrame(() => { + this.flushPendingTrailUpdates(); + }); + } + } + + private flushPendingTrailUpdates() { + this.trailUpdateRafId = null; + for (const [callsign, isHistorical] of this.pendingTrailUpdates) { + const trailState = this.trails.get(callsign); + if (trailState) { + this.updateTrailVisualization(callsign, trailState, isHistorical); + } + } + this.pendingTrailUpdates.clear(); } private extractBaseCallsign(markerId: string | number): string { @@ -532,6 +556,14 @@ export class TrailManager { }); } + destroy() { + if (this.trailUpdateRafId !== null) { + cancelAnimationFrame(this.trailUpdateRafId); + this.trailUpdateRafId = null; + } + this.pendingTrailUpdates.clear(); + } + cleanupOldPositions() { // Don't clean up historical positions - only clean up very old live positions const veryOldCutoff = Date.now() - 24 * 60 * 60 * 1000; // 24 hours diff --git a/assets/js/map.ts b/assets/js/map.ts index ec3386c..9c56cde 100644 --- a/assets/js/map.ts +++ b/assets/js/map.ts @@ -582,14 +582,13 @@ let MapAPRSMap = { ? Math.abs(currentZoom - self.lastZoom) : 0; - // Handle OMS markers when crossing zoom threshold - if (self.oms) { - // Clear and re-add all markers to OMS on zoom change + // Only rebuild OMS when crossing the clustering threshold + const wasUnclustered = self.lastZoom !== undefined && self.lastZoom >= DISABLE_CLUSTERING_AT_ZOOM; + const isUnclustered = currentZoom >= DISABLE_CLUSTERING_AT_ZOOM; + if (self.oms && (wasUnclustered !== isUnclustered || self.lastZoom === undefined)) { self.oms.clearMarkers(); self.markers.forEach((marker, id) => { const markerState = self.markerStates.get(String(id)); - // Only add most recent markers (those with icons) to OMS for spidering - // Fallback: if is_most_recent_for_callsign is undefined, exclude historical markers as before const shouldAddToOms = markerState?.is_most_recent_for_callsign === true || (markerState?.is_most_recent_for_callsign == null && @@ -1176,18 +1175,17 @@ let MapAPRSMap = { const serverConvertKeys = payload.convert_to_historical; let markersToConvert: string[] = []; - // Build set of new packet IDs to avoid converting a marker we're about to add + // Single pass: build new IDs set and callsigns to convert const allNewIds = new Set(); - for (const data of packets) { - if (data.id) allNewIds.add(String(data.id)); - } - - // Determine which callsigns need conversion const callsignsToConvert: Set = new Set(); if (serverConvertKeys && serverConvertKeys.length > 0) { for (const cs of serverConvertKeys) callsignsToConvert.add(cs); + for (const data of packets) { + if (data.id) allNewIds.add(String(data.id)); + } } else { for (const data of packets) { + if (data.id) allNewIds.add(String(data.id)); const cs = data.callsign_group || data.callsign || data.id; if (cs) callsignsToConvert.add(cs); } @@ -1909,22 +1907,12 @@ let MapAPRSMap = { } // If marker._oms is true but no _omsData, let OMS handle it (will spiderfy) - // Bring the clicked marker to front + // Bring the clicked marker to front using a counter instead of DOM scan if (marker.getElement) { const element = marker.getElement(); if (element) { - // Find the highest z-index among all markers - let maxZIndex = 1000; - document.querySelectorAll(".leaflet-marker-icon").forEach((el) => { - const zIndex = parseInt( - (el as HTMLElement).style.zIndex || "0", - 10, - ); - if (zIndex > maxZIndex) maxZIndex = zIndex; - }); - - // Set this marker's z-index higher than all others - element.style.zIndex = (maxZIndex + 1).toString(); + self.markerZIndexCounter = (self.markerZIndexCounter || 1000) + 1; + element.style.zIndex = self.markerZIndexCounter.toString(); } } @@ -2380,19 +2368,22 @@ let MapAPRSMap = { // If this is the most recent for the callsign, always show the APRS icon if (data.is_most_recent_for_callsign) { // Remove any historical dot at the same position for this callsign - if (self.markers && self.markerStates) { - self.markers.forEach((marker: APRSMarker, id: string) => { - const state = self.markerStates!.get(String(id)); - if ( - state && - state.historical && - state.callsign_group === data.callsign_group && - Math.abs(state.lat - data.lat) < 0.00001 && - Math.abs(state.lng - data.lng) < 0.00001 - ) { - self.removeMarkerWithoutTrail(id); + // Use callsignIndex for O(1) lookup instead of scanning all markers + if (self.markers && self.markerStates && self.callsignIndex && data.callsign_group) { + const ids = self.callsignIndex.get(data.callsign_group); + if (ids) { + for (const id of ids) { + const state = self.markerStates.get(String(id)); + if ( + state && + state.historical && + Math.abs(state.lat - data.lat) < 0.00001 && + Math.abs(state.lng - data.lng) < 0.00001 + ) { + self.removeMarkerWithoutTrail(String(id)); + } } - }); + } } // Use server-generated symbol HTML if available if (data.symbol_html) { @@ -2482,6 +2473,11 @@ let MapAPRSMap = { self.cleanupTimeouts = []; } + // Clean up trail manager + if (self.trailManager) { + self.trailManager.destroy(); + } + // Remove popup navigation event listener if (self.popupNavigationHandler) { document.removeEventListener("click", self.popupNavigationHandler); diff --git a/assets/js/map_helpers.ts b/assets/js/map_helpers.ts index 4523cda..f190123 100644 --- a/assets/js/map_helpers.ts +++ b/assets/js/map_helpers.ts @@ -88,15 +88,6 @@ export function saveMapState(map: L.Map, pushEvent: PushEventFunction) { const truncatedLat = Math.round(center.lat * 100000) / 100000; const truncatedLng = Math.round(center.lng * 100000) / 100000; - // Always save to localStorage, even if LiveView is disconnected - // Temporarily disabled - using server-side geolocation instead - /* - localStorage.setItem( - "aprs_map_state", - JSON.stringify({ lat: truncatedLat, lng: truncatedLng, zoom }), - ); - */ - // Send combined map state update to server for URL and bounds updating const payload = { center: { lat: truncatedLat, lng: truncatedLng }, diff --git a/lib/aprsme/cleanup_scheduler.ex b/lib/aprsme/cleanup_scheduler.ex index 7907e9c..cc32ef6 100644 --- a/lib/aprsme/cleanup_scheduler.ex +++ b/lib/aprsme/cleanup_scheduler.ex @@ -37,8 +37,7 @@ defmodule Aprsme.CleanupScheduler do def handle_info(:schedule_cleanup, %{interval: interval} = state) when is_integer(interval) do Logger.info("Running packet cleanup task") - # Run cleanup directly in a supervised task - Task.start(fn -> + Aprsme.BroadcastTaskSupervisor.async_execute(fn -> try do PacketCleanupWorker.perform(%{}) rescue diff --git a/lib/aprsme/is/is_supervisor.ex b/lib/aprsme/is/is_supervisor.ex index e4407a7..9837a9e 100644 --- a/lib/aprsme/is/is_supervisor.ex +++ b/lib/aprsme/is/is_supervisor.ex @@ -12,8 +12,7 @@ defmodule Aprsme.Is.IsSupervisor do {Aprsme.Is, []} ] - # Supervisor will restart children max 3 times in 5 seconds - # This prevents rapid reconnection attempts - Supervisor.init(children, strategy: :one_for_one, max_restarts: 3, max_seconds: 5) + # Allow more restarts over a longer window to survive transient network issues + Supervisor.init(children, strategy: :one_for_one, max_restarts: 5, max_seconds: 60) end end diff --git a/lib/aprsme/packet_consumer.ex b/lib/aprsme/packet_consumer.ex index 717ce63..2cd09e4 100644 --- a/lib/aprsme/packet_consumer.ex +++ b/lib/aprsme/packet_consumer.ex @@ -248,17 +248,6 @@ defmodule Aprsme.PacketConsumer do }, %{} ) - - # Logger.info("Batch processing completed", - # batch_result: - # LogSanitizer.log_data( - # packet_count: length(packets), - # duration_ms: duration, - # success_count: success_count, - # error_count: error_count, - # memory_diff_bytes: memory_diff - # ) - # ) end @spec process_chunk(list(map())) :: {non_neg_integer(), non_neg_integer()} diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 617fc64..be5f5ca 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -227,11 +227,6 @@ defmodule Aprsme.Packets do case %Packet{} |> Packet.changeset(attrs) |> Repo.insert() do {:ok, packet} -> - # Invalidate cache for this packet's callsign - if Map.has_key?(attrs, :sender) do - # Cache invalidation removed - no longer using CachedQueries - end - {:ok, packet} {:error, changeset} -> @@ -808,11 +803,16 @@ defmodule Aprsme.Packets do """ @spec get_latest_weather_packet(String.t()) :: struct() | nil def get_latest_weather_packet(callsign) when is_binary(callsign) do - # Use proper index with short time window first - case get_latest_weather_in_window(callsign, 24) do - nil -> get_latest_weather_in_window(callsign, 168) - packet -> packet - end + import Ecto.Query + + query = + from p in Packet, + where: p.sender == ^callsign, + where: p.has_weather == true, + order_by: [desc: p.received_at], + limit: 1 + + Repo.one(query) end @doc """ @@ -879,22 +879,4 @@ defmodule Aprsme.Packets do } end) end - - defp get_latest_weather_in_window(callsign, hours) do - import Ecto.Query - - since = DateTime.add(DateTime.utc_now(), -hours * 3600, :second) - - query = - from p in Packet, - where: p.sender == ^callsign, - where: p.received_at > ^since, - where: - not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or - not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), - order_by: [desc: p.received_at], - limit: 1 - - Repo.one(query) - end end diff --git a/lib/aprsme_web/endpoint.ex b/lib/aprsme_web/endpoint.ex index d863587..45dc65a 100644 --- a/lib/aprsme_web/endpoint.ex +++ b/lib/aprsme_web/endpoint.ex @@ -28,7 +28,7 @@ defmodule AprsmeWeb.Endpoint do plug Plug.Static, at: "/", from: :aprsme, - gzip: false, + gzip: true, only: ~w(assets fonts images aprs-symbols favicon.ico robots.txt) if Mix.env() == :dev do diff --git a/lib/aprsme_web/router.ex b/lib/aprsme_web/router.ex index f9890d3..7b0d92e 100644 --- a/lib/aprsme_web/router.ex +++ b/lib/aprsme_web/router.ex @@ -15,10 +15,10 @@ defmodule AprsmeWeb.Router do plug :put_root_layout, {AprsmeWeb.Layouts, :root} plug :protect_from_forgery - # plug :put_secure_browser_headers, %{ - # "content-security-policy" => - # "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://js.sentry-cdn.com https://unpkg.com https://cdn.jsdelivr.net https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://unpkg.com; img-src 'self' data: https: http: blob:; font-src 'self' data:; connect-src 'self' wss: https://*.ingest.sentry.io https://*.sentry.io https://nominatim.openstreetmap.org https://tile.openstreetmap.org https://*.tile.openstreetmap.org https://*.tile.openstreetmap.de https://*.basemaps.cartocdn.com; media-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; frame-src 'self'; manifest-src 'self'; worker-src 'self' blob:" - # } + plug :put_secure_browser_headers, %{ + "content-security-policy" => + "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://js.sentry-cdn.com https://unpkg.com https://cdn.jsdelivr.net https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://unpkg.com; img-src 'self' data: https: http: blob:; font-src 'self' data:; connect-src 'self' wss: https://*.ingest.sentry.io https://*.sentry.io https://nominatim.openstreetmap.org https://tile.openstreetmap.org https://*.tile.openstreetmap.org https://*.tile.openstreetmap.de https://*.basemaps.cartocdn.com; media-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; frame-src 'self'; manifest-src 'self'; worker-src 'self' blob:" + } plug :fetch_current_user plug AprsmeWeb.Plugs.SetLocale