slim icon indicating copy to clipboard operation
slim copied to clipboard

[BUG] Extreme memory usage during rendering

Open shivabhusal opened this issue 7 years ago • 2 comments

The following slim partial used 38GB RAM in Mac and 8GB in linux.

.drivers-view
  h2.title Drivers
  .actions
    //= link_to 'Add New Driver', new_driver_path, class: 'button'
    button.add onclick="window.location.href='/drivers/new'"
      ' Add new driver
    button#saveDriverButton disabled="true"
      '  Save

  - if @drivers.empty?
    p Your organization has no drivers.
  - else
    .table-box
      .table-scroll
        table
          thead
            tr
              th.text-center width="50" ID
              th Name
              th Image
              th Phone
              th OTP
              th Preferred base
              th Available time
              th Available days
              th Status
              th.text-center width="80" Actions
          tbody
            - @drivers.each do |driver|
              tr
                td
                  = driver.id
                td
                  = driver.name
                td
                  .driver-image
                    = image_tag driver.avatar.url(:medium), alt:( (driver.name) ? driver.name[0..1] : 'N/A')
                td
                  = driver.phone
                td
                  = driver.otp
                td.preferred-hub
                  = select_tag 'preferred_hub',  options_from_collection_for_select(::Base.sorted, 'id', 'name', driver.preferred_base_id.try(:to_s)),
                              class: 'custom-select', data: {worker_id: driver.id},
                              prompt: 'Select a Hub'
                  //i.fas.fa-angle-down
                td
                  = driver.time_available
                td
                  = driver.days_available
                td
                  - if driver.status
                    .label class=driver.status
                      = driver.status.humanize
                td.text-center
                  = link_to edit_driver_path(driver)
                    i.fas.fa-pencil-alt
                  = link_to generate_otp_driver_path(driver), class: 'margin15left', title: 'Regenerate OTP', method: :put do
                    i.fa.fa-sync

Problem

                  = if driver.status
                    .label class=driver.status
                      = driver.status.humanize

Solution

                 - if driver.status
                    .label class=driver.status
                      = driver.status.humanize

I have used = instead of - before if.

Conclusion

Its fixed now, but I expect SLIM to handle this

shivabhusal avatar Nov 30 '18 10:11 shivabhusal

I can confirm some performance issues as well. But they are not caused by = if and I could haven't figured out yet why.

deepj avatar Dec 03 '18 12:12 deepj

I've run into what I think is another manifestation of the same problem...

Let's say you have a helper method defined:

def link_to_yielded_url(link_text)
  url = yield
  link_to(link_text, url)
end

(With or without a &block argument.) Then the usage might look like:

= link_to_yielded_url("Login")
  - if locale == 'es-ES'
    = login_url(country: 'es')
  - else
    = login_url(country: 'fr')

(The specifics here are made up.)

This kind of helper leaks memory, and if you use it inside of an each block, dozens or thousands of times, it starts to cause the kind of out-of-control memory consumption that @shivabhusal described.

If the nested block doesn't render (using =), then this memory issue doesn't occur:

= link_to_yielded_url("Login")
  - if locale == 'es-ES'
    - login_url(country: 'es')
  - else
    - login_url(country: 'fr')

The block still gets evaluated when yield is called. Using capture in the method also prevents the issue:

def link_to_yielded_url(link_text, &block)
  url = capture(&block)
  link_to(link_text, url)
end

This works correctly and safely whether you use - or = in the slim contents of &block.

So I guess this is ultimately a syntactic nuance, but the impact of it on memory consumption could be subtle and hard to pin down, or in the wrong circumstances, it could grind everything to a halt. Since memory consumption is the only observable difference in the result, it would be better if this could be handled safely by default.

nbdavies avatar Dec 02 '20 23:12 nbdavies