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

Improve Performance of Daily Docket Page #14594

Merged
merged 21 commits into from
Jun 29, 2020
Merged

Conversation

ferristseng
Copy link
Contributor

@ferristseng ferristseng commented Jun 24, 2020

Resolves #14432

Description

Performance Improvements:

  • Update LegacyHearing#quick_to_hash to actually call the quick hasher
  • Load POA details async on Daily Docket. This allows us to skip hashing it on the backend (avoiding a call to BGS), and hide the Caseflow-loading-logo more quickly
  • For quick hasher, remove email addresses, email events, and representative info. Those can all trigger BGS calls
  • Prevent DailyDocketContainer from loading hearings twice. Hearings are returned with the hearing day, but also the full details are async loaded. This removes fetching each individual hearing async.

I mainly tried to focus on trying to reduce unnecessary calls to BGS. New Relic was showing that the bulk of processing time was spent waiting for BGS (top 2 categories):

Screen Shot 2020-06-23 at 1 47 11 PM

I removed calls to fetch associated data that we didn't need on the daily docket page, especially data that could trigger a call to BGS like email address data. One thing I couldn't do was remove the BGS call to get representative details, but we did have components that could load it async, which I ended up using to make it appear the page was loading a bit faster.

Benchmarks

Before

#---#
2020-03-12 ID: 4644
2020-06-23 ID: 5499

Benchmark.measure do
  HearingDay.find(4644)
    .hearings_for_user(User.system_user)
    .map { |h| h.quick_to_hash(1217) }
end
#---#

4644 => #<Benchmark::Tms:0x000000000ed580c8 @label="", @real=43.06742242600012, @cstime=0.0, @cutime=0.0, @stime=0.15835700000000008, @utime=1.6796589999999991, @total=1.8380159999999992>

5499 => #<Benchmark::Tms:0x000000000f00bb58 @label="", @real=30.49937072099965, @cstime=0.0, @cutime=0.0, @stime=0.15402099999999996, @utime=1.3248880000000005, @total=1.4789090000000005>

After

#---#
2020-03-12 ID: 4644
2020-06-23 ID: 5499

Benchmark.measure do
  HearingDay.find(4644)
    .hearings_for_user(User.system_user)
    .map do |h|
      if h.is_a?(LegacyHearing)
        ::LegacyHearingSerializer.quick(
          h,
          params: { current_user_id: User.system_user.id }
        ).serializable_hash[:data][:attributes]
      else
        h.quick_to_hash(1217)
      end
    end
end
#---#

4644 => #<Benchmark::Tms:0x000000000e64b960 @label="", @real=3.9002713530007895, @cstime=0.0, @cutime=0.0, @stime=0.04220700000000033, @utime=0.4883059999999997, @total=0.530513>

5499 => #<Benchmark::Tms:0x000000000ebffac8 @label="", @real=1.703293478998603, @cstime=0.0, @cutime=0.0, @stime=0.043072999999999695, @utime=0.4282699999999995, @total=0.4713429999999992>

Results

Using real time:

43.06742242600012 seconds -> 3.9002713530007895 seconds
30.49937072099965 seconds -> 1.703293478998603 seconds

Acceptance Criteria

  • Verify backend performance gains

Code Documentation Updates

  • Add or update code comments at the top of the class, module, and/or component.

Storybook Story

For Frontend (Presentationa) Components

  • Add a Storybook file alongside the component file (e.g. create MyComponent.stories.js alongside MyComponent.jsx)
  • Give it a title that reflects the component's location within the overall Caseflow hierarchy
  • Write a separate story (within the same file) for each discrete variation of the component

@va-bot
Copy link
Collaborator

va-bot commented Jun 24, 2020

1 Warning
⚠️ This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

@codeclimate
Copy link

codeclimate bot commented Jun 24, 2020

Code Climate has analyzed commit 6ad899e and detected 0 issues on this pull request.

View more on Code Climate.

@ferristseng ferristseng requested review from a team and rubaiyat22 and removed request for a team and rubaiyat22 June 25, 2020 18:17
Copy link
Contributor

@rubaiyat22 rubaiyat22 left a comment

Choose a reason for hiding this comment

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

Just noticed that the power of attorney detail in case details page has some extra line spaces which wasn't introduced by your PR but I'm wondering if you know this has always been like this.

Power of attorney detail compared to Veteran detail

Screen Shot 2020-06-29 at 11 20 12 AM |
Screen Shot 2020-06-29 at 11 20 16 AM

@@ -8,14 +8,6 @@ export const dailyDocketReducer = function(state = {}, action = {}) {
hearingDay: { $set: action.payload.hearingDay },
hearings: { $set: action.payload.hearings }
});
case ACTIONS.RECEIVE_HEARING:
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! one less reducer

Copy link
Contributor

@rubaiyat22 rubaiyat22 left a comment

Choose a reason for hiding this comment

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

LGTM!!! left a comment about the power of attorney detail

@ferristseng
Copy link
Contributor Author

Just noticed that the power of attorney detail in case details page has some extra line spaces which wasn't introduced by your PR but I'm wondering if you know this has always been like this.

Power of attorney detail compared to Veteran detail

Screen Shot 2020-06-29 at 11 20 12 AM |
Screen Shot 2020-06-29 at 11 20 16 AM

I have not noticed this, but that is a good catch! I can see if I can tweak the styling it in this PR too

@ferristseng
Copy link
Contributor Author

Resolved! Thank you for catching that:

Screen Shot 2020-06-29 at 11 53 21 AM

@ferristseng ferristseng added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Jun 29, 2020
@va-bot va-bot merged commit 63b00c0 into master Jun 29, 2020
@va-bot va-bot deleted the ftseng-performance-daily-docket branch June 29, 2020 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Tango 💃
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate performance issues on Daily Docket
3 participants