Skip to content

refactor(DotcomWeb.ModeView): simpler has_alert? - #3379

Merged
thecristen merged 5 commits into
mainfrom
cbj/route-has-alert
Jul 31, 2026
Merged

refactor(DotcomWeb.ModeView): simpler has_alert?#3379
thecristen merged 5 commits into
mainfrom
cbj/route-has-alert

Conversation

@thecristen

@thecristen thecristen commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Scope

The homepage's "Recently Visited" section shows routes. If a route has an important alert, we show our familiar triangle warning icon.

image

My local load testing on the homepage has found that fetching and processing alerts is by far the most computationally intensive part of the homepage. The "Recently Visited" buttons are a small part of what we do with homepage alerts, but I wanted to see if I could make it a bit more performant.

Implementation

Instead of having the homepage fetch every alert and then having the view check the list of all alerts for whether a given route is present, I wrote a helper function to more directly assess whether a route has a high priority alert.

The first two commits assemble the needed queries:

feat(Alerts.Cache.Store): get from ids + priority

We already have functions to easily and performantly get a list of alert IDs associated with a route ID. I wanted to build upon that by making it just as easy to find which of those alerts have high priority.

feat(Alerts.Repo): get by route ID + priority

This uses the helper function from the last commit to make it easy to fetch all alerts for a given route ID & priority.

refactor(DotcomWeb.ModeView): simpler has_alert?

Using the new Alerts.Repo function in place of the existing logic. We still have to narrow it down to the currently active ones.

Since this changes the signature of DotcomWeb.ModeView.has_alert?/3 a bunch, I got to adjust a bunch of code calling it! As an aside... I have a near-term goal of having the homepage not need a huge list of all @alerts at all, and this'll help a lot 😃

Note

I changed the underlying logic slightly -- the previous version used Alerts.Alert.high_severity_or_high_priority?/1, which is actually inconsistent with the rest of the website.

It also struck me as extraneous, as the logic which sets priority to :high already incorporates high severity through this block...

def priority(%{severity: severity} = params, time) when severity >= 7 do
   if urgent?(params, time), do: :high, else: :low
end

That was also the only place Alerts.Alert.high_severity_or_high_priority?/1 was invoked, so removing that call means I could delete that function too.

Screenshots

No changes!

How to test

I extracted some flamegraphs... the middle section changed a lot 😅

Click to see them yourself *note these flamegraphs omit the extra work done to render the alerts tab, because `FlameOn` kept timing out
BeforeAfter
recentlyvisitedbefore recentlyvisitedafter
Click here to see Claude's interpretation of them

The big finding: _grid_buttons.html was spawning Tasks per render

In the before flamegraph, DotcomWeb.ModeView rendering the grid buttons partial goes through this call chain:

_grid_buttons.html → Enum.with_index → Enum.map_reduce → Task.Supervised.stream (x6 instances)
                                                              ├─ Task.Supervised.stream_reduce/7  (~58ms each)
                                                              └─ Task.Supervised.stream_deliver/7  (~58ms each)

That's Task.async_stream (or similar) being used inside a template partial — spawning and supervising a separate BEAM process per grid button just to do (presumably) small, cheap work like resolving an icon or route link. In the after version, that entire code path is gone: the same partial now costs ~250µs total instead of tens of milliseconds per task. This alone accounts for roughly 58ms of the total 65ms improvement.

Overall numbers

BeforeAfter
Total render time 262.6ms197.0ms
Improvement ~25% faster (-65.6ms)

@thecristen
thecristen requested a review from a team as a code owner July 31, 2026 14:02
@thecristen
thecristen requested a review from lvachon1 July 31, 2026 14:02

@lvachon1 lvachon1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, runs great, and I learned some new syntax tricks from reading this PR. Thanks!

@thecristen
thecristen merged commit aa2a308 into main Jul 31, 2026
19 checks passed
@thecristen
thecristen deleted the cbj/route-has-alert branch July 31, 2026 15:17
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