ElMassimo / vite_ruby

⚡️ Vite.js in Ruby, bringing joy to your JavaScript experience
https://vite-ruby.netlify.app/
MIT License
1.28k stars 117 forks source link

resolve_entries is slow when using vite_javascript_tag for very large and complex tsx react/relay files #416

Open SemmyTan opened 10 months ago

SemmyTan commented 10 months ago

Description 📖

Provide a clear and concise description of what the bug is.

The method resolve_entries in lib/vite_ruby/manifest.rb (https://github.com/ElMassimo/vite_ruby/blob/main/vite_ruby/lib/vite_ruby/manifest.rb#L27) is very slow when it is called from the vite_rails method vite_javascript_tag for a very complex .tsx file that uses react and relay modern. The .tsx file in my case imports many levels of nested children components - the resulting output from the single vite_javascript_tag method call is over 300 <link rel="modulepreload" ... tags and a few dozen stylesheet tags. The actual web app runs smoothly so there aren't any performance issues that I can see with it.

The cause of the slowness in my case is the .uniq at the end of this line in the resolve_entries method:

imports = dev_server_running? ? [] : entries.flat_map { |entry| entry['imports'] }.compact.uniq

I inspected the object that .uniq was being called for in my case and it was an array of over 300 hashes. Some of these hashes were gigantic - the 5 largest ones were between 100 and 300 million characters long when I ran .inspect.length on the hashes. .uniq seems to be very slow when comparing multiple gigantic hashes - the run time in my case varied between 6 to 12 seconds depending on the specs of the hardware I was testing with.

The fix for my case was simple - I just removed the .uniq at the end of that line. The 2 spots the imports variable is used further below in that method both have their own .uniq call on the resulting value, so the first .uniq when defining imports = ... does not seem necessary. I added this monkey patch to my code base, but thought it might be a useful change to add to the vite gem if there aren't any problems with this change. I can make a pull request for this change if it seems fine.

Reproduction 🐞

Please provide a link to a repo that can reproduce the problem you ran into.

Unfortunately, I am not able to share the code base where this is happening and creating a new repository large and complex enough to replicate the issue does not seem to be a trivial task.

Logs 📜

If not providing a reproduction:

Output I added logging to check the length of the hashes. In the private code base I am working with, this resulted in an array of over 300 numbers. The 5 largest ones were between 100 and 300 million characters long each. Summing the lengths resulted in 1.3 billion. I know `.inspect` includes all the syntax characters such as `{` which causes this to be longer than the real data, but numbers this large still show that the hashes are ridiculously large. ``` ... imports = dev_server_running? ? [] : entries.flat_map { |entry| entry['imports'] }.compact.uniq Rails.logger.info imports.map{|hash| hash.inspect.to_s.length}.sort Rails.logger.info imports.map{|hash| hash.inspect.to_s.length}.sum # outputs 1314704174 ... ```
ElMassimo commented 10 months ago

Hi Semmy, thanks for reporting.

Given that we call uniq on the JS imports and the CSS stylesheets later on, it sounds reasonable to remove the uniq call on line 31.

ashaninBenjamin commented 7 months ago

We've just faced the same issue. It takes about 9s to take unique elements. However, there are only 127 entries on the 1st level. The manifest file has 12k lines, it's not enormous.

We added a monkey patch to avoid using the Array#uniq method.