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

Slow Loading in Wallaby 7.0 #222

Open
ryanb opened this issue Apr 26, 2024 · 9 comments
Open

Slow Loading in Wallaby 7.0 #222

ryanb opened this issue Apr 26, 2024 · 9 comments

Comments

@ryanb
Copy link

ryanb commented Apr 26, 2024

@tian-im Upgrading to 7.0.0 causes every wallaby page load to be 6+ seconds in development and test. Haven't tested in production/staging. Loading is normal in beta2.

I may not have time to debug this today but will let you know when I figure it out. Do you have any ideas what could cause it off the top of your head?

@tian-im
Copy link
Collaborator

tian-im commented Apr 27, 2024

Hmmm, the differences between 7.0.0beta2 and 7.0.0 are these MRs:

Have you recently created a new file under app/models which might be stuck in an infinite loading loop?

@ryanb
Copy link
Author

ryanb commented Apr 29, 2024

@tian-im

Have you recently created a new file under app/models which might be stuck in an infinite loading loop?

This happens after restarting the rails server and reloading the admin pages a few times, every time it's 6+ seconds.

I'll try to look into this more on Friday.

@ryanb
Copy link
Author

ryanb commented May 3, 2024

@tian-im I did some profiling and the issue is the e.message call here.

When calling the exception message it does a DidYouMean check which takes a few milliseconds. These add up due to the large number of class checks. Here's a picture of the flame graph.

Screenshot 2024-05-03 at 3 38 18 PM

Looks like we won't be able to check the exception message here.

@tian-im
Copy link
Collaborator

tian-im commented May 4, 2024

@ryanb I see, I was thinking of caching the ones already checked. Let me see how I can improve this.

@tian-im
Copy link
Collaborator

tian-im commented May 5, 2024

@ryanb I've merged this PR #223, could you give it a try?

git 'https://github.com/wallaby-rails/wallaby-rails.git' do
  gem "wallaby"
  gem "wallaby-core"
  gem "wallaby-view"
  gem "wallaby-active_record"
end

@ryanb
Copy link
Author

ryanb commented May 6, 2024

Thanks, I'll give it a try on Friday.

@ryanb
Copy link
Author

ryanb commented May 10, 2024

@tian-im it seems the first page load after the server starts is still slow but subsequent loads are faster. Makes sense if you're doing caching. This will work ok for development, but is this something that would be experienced in production as well though? I wonder about tests as well, hmm.

Also there appears to be an issue with loading models with namespaces. I have a model Account::Invoice and it works in 7.0.0beta2 but not in latest main. I get a 404 error when I click on it in the nav bar.

Are both of these issues due to fixing #220? If so maybe it should just go back to ignoring exceptions for now.

@tian-im
Copy link
Collaborator

tian-im commented May 12, 2024

@ryanb Do you have more details about the Account::Invoice? e.g.

  • What's its relative file path?
  • Is Account a class or module?
  • What's the output of Wallaby::ResourcesRegexp.new.execute? Does it include account/invoices?

I will change to ignore the exception if it's not configured to raise it

@ryanb
Copy link
Author

ryanb commented May 13, 2024

@ryanb Do you have more details about the Account::Invoice? e.g.

Sorry I mistyped, it's Accounts::Invoice (plural Accounts).

  • What's its relative file path?

app/models/accounts/invoice.rb

  • Is Account a class or module?

It's a module.

# app/models/accounts.rb
module Accounts
  def self.table_name_prefix
    "accounts_"
  end
end
  • What's the output of Wallaby::ResourcesRegexp.new.execute? Does it include account/invoices?

Yes.

>> Wallaby::ResourcesRegexp.new.execute
=> 
/(zapier_subscription\/data_point_triggers|views\/feedbacks|views|view_position_sharings|view_position_ownerships|view_hits|vanta_integrations|users\/otp_sessions|users\/otp_recovery_codes|users\/otp_configs|users\/new_experiences|users\/email_verifications|users|user_auth_tokens|trends|trend_alert_flags|teams|team_hierarchies|team_extractions|team_charts\/feedbacks|team_charts|tasks|task_hierarchies|task_dependencies|sub_graph_associations|sub_graph_association_hierarchies|stripe_billings|slack_integrations|shared_links|search\/visits|search\/records|search\/queries|saml_configs|s3_integrations|roles|role_attachments|referred_accounts|recent_graph_displays|positions|position_groups|position_group_member_associations|position_group_hierarchies|position_group_filters|position_graph_group_associations|position_graph_associations|permissions|permission_member_associations|permission_graph_exclusions|permission_graph_associations|permission_data_export_types|paper_trail\/versions|organisations|organisation_mailings\/previous_missing_values_mailings|organisation_mailings\/objectives_mailings|organisation_mailings\/knowledge\/digest_mailings|organisation_mailings\/knowledge\/assignments_upcoming_mailings|organisation_mailings\/knowledge\/assignments_overdue_mailings|organisation_mailings\/custom_mailings|organisation_mailings\/current_missing_values_mailings|organisation_mailings\/base_mailings|organisation_mailings\/any_missing_values_mailings|organisation_hierarchies|objectives|objective_proposals|objective_hierarchies|notifications|notification_types|notification_settings|notification_recipients|members\/mailing_settings|members|member_away_periods|mailing_schedules|mailing_deliveries|knowledge\/suggestions|knowledge\/suggestion_replies|knowledge\/subjects|knowledge\/subject_hierarchies|knowledge\/subject_assignments|knowledge\/responsibilities|knowledge\/rejections|knowledge\/publication_notices|knowledge\/items|knowledge\/item_versions|knowledge\/item_version_subject_associations|knowledge\/item_version_hierarchies|knowledge\/item_types|knowledge\/item_search_indices|knowledge\/item_reviews|knowledge\/item_responsibility_marks|knowledge\/item_nestings|knowledge\/item_nesting_hierarchies|knowledge\/glossary_entries|knowledge\/favorites|knowledge\/distinct_assignments|knowledge\/contents|knowledge\/configs|knowledge\/comments|knowledge\/comment_subscriptions|knowledge\/comment_replies|knowledge\/comment_notices|knowledge\/collaborator_infos|knowledge\/collaborations|knowledge\/collaboration_requests|knowledge\/attestations|knowledge\/attachments|knowledge\/assignments|knowledge\/approvals|knowledge\/approval_steps|knowledge\/approval_role_associations|knowledge\/approval_requests|knowledge\/approval_permits|knowledge\/activities|key_results|guests|growth_league\/weeks|growth_league\/values|growth_league\/scores|growth_league\/rounds|growth_league\/graph_assignments|growth_league\/enrollments|graphs\/yearly_graphs|graphs\/weekly_graphs|graphs\/quarterly_graphs|graphs\/monthly_graphs|graphs\/graphs|graphs\/daily_graphs|graphs\/concatenated_graphs|graphs\/comparison_graphs|graphs\/calculated_graphs|graphs\/automatic_graphs|graph_templates|graph_targets|graph_quotas|graph_groups|graph_group_permissions|graph_group_associations|glossary\/tags|glossary\/taggings|events|event_types|email_templates|data_points|data_point_imports|data_imports|data_exports|custom_attributes|custom_attribute_fields|api_requests|alerts|alert_types|alert_team_associations|alert_position_associations|admin_users|active_record\/session_store\/sessions|accounts\/invoices|accounts|account_holderships) # all the possible resources names
/x

I will change to ignore the exception if it's not configured to raise it

Thanks! I can also change the config on my end if you prefer to keep it enabled by default. The performance may only be an issue on large apps.

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

No branches or pull requests

2 participants