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

Fix textareas only after preserved timing #941

Merged
merged 1 commit into from Jul 14, 2017

Conversation

Projects
None yet
1 participant
@k0kubun
Member

k0kubun commented Jul 11, 2017

Description

We call Haml::Buffer#fix_textareas! to rollback textarea content's first 
, which is unexpectedly preserved from \n by Haml::Helpers#preserve, to \n for every script.

But it means that we need to call Haml::Buffer#fix_textareas! only after we call preserve. So I fixed to do so.

Benchmark

With Ruby 2.4.0 and k0kubun/haml_bench/templates/slim_bench.haml,

before

Calculating -------------------------------------
          haml 4.0.7     41.534k (± 1.4%) i/s -    210.652k in   5.072717s
          haml 5.0.1    138.067k (± 1.5%) i/s -    700.872k in   5.077348s

Comparison:
          haml 5.0.1:   138067.3 i/s
          haml 4.0.7:    41534.1 i/s - 3.32x  slower

after

Calculating -------------------------------------
          haml 4.0.7     39.466k (± 2.1%) i/s -    199.206k in   5.049870s
          haml 5.0.1    148.617k (± 1.9%) i/s -    750.109k in   5.049201s

Comparison:
          haml 5.0.1:   148616.6 i/s
          haml 4.0.7:    39466.1 i/s - 3.77x  slower

Now _hamlout's dependency is only _hamlout.attributes call for Hash in that benchmark template. It means that we have new optimization chance to lazily initialize Haml::Buffer instance, which is the current bottleneck of Haml.

@@ -159,7 +159,7 @@ def rstrip!
# @since Haml 4.0.1
# @private
def fix_textareas!(input)
return input unless toplevel? && input.include?('<textarea'.freeze)

This comment has been minimized.

@k0kubun

k0kubun Jul 11, 2017

Member

Note that toplevel? is considered to be added just for optimization, and it must be removed now to apply this in partial view (we didn't need to call in partial before because outside layout will render partial as ruby script and call fix_textareas! for it).

And this fix won't cause regression as this method is idempotent and safe to call multiple times.

@k0kubun

k0kubun Jul 11, 2017

Member

Note that toplevel? is considered to be added just for optimization, and it must be removed now to apply this in partial view (we didn't need to call in partial before because outside layout will render partial as ruby script and call fix_textareas! for it).

And this fix won't cause regression as this method is idempotent and safe to call multiple times.

@k0kubun

This comment has been minimized.

Show comment
Hide comment
@k0kubun

k0kubun Jul 11, 2017

Member

As the original problem is heavily tested for both Rails and non-Rails templates and tests pass, I think this is safe to merge.

@haml/committers Thoughts?

Member

k0kubun commented Jul 11, 2017

As the original problem is heavily tested for both Rails and non-Rails templates and tests pass, I think this is safe to merge.

@haml/committers Thoughts?

@k0kubun

This comment has been minimized.

Show comment
Hide comment
@k0kubun

k0kubun Jul 14, 2017

Member

Since this patch does not introduce any behavior change, merging this for now. Please fell free to add some feedback later.

Member

k0kubun commented Jul 14, 2017

Since this patch does not introduce any behavior change, merging this for now. Please fell free to add some feedback later.

@k0kubun k0kubun merged commit ece8de7 into haml:master Jul 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@k0kubun k0kubun deleted the k0kubun:textarea-limited-fix branch Jul 14, 2017

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 10, 2017

taca
Update ruby-haml to 5.0.3.
## 5.0.3

Released on September 7, 2017
([diff](haml/haml@v5.0.2...v5.0.3)).

* Use `String#dump` instead of `String#inspect` to generate string
  literal. (Takashi Kokubun)
* Fix Erubi superclass mismatch
  error. [#952](haml/haml#952) (thanks [Robin
  Daugherty](https://github.com/RobinDaugherty))

## 5.0.2

Released on August 1, 2017
([diff](haml/haml@v5.0.1...v5.0.2)).

* Let `haml -c` fail if generated Ruby code is syntax
  error. [#880](haml/haml#880) (Takashi Kokubun)
* Fix `NoMethodError` bug caused with Sprockets 3 and :sass
  filter. [#930](haml/haml#930) (thanks [Gonzalez
  Maximiliano](https://github.com/emaxi))
* Fix `list_of` helper with multi-line
  content. [#933](haml/haml#933) (thanks [Benoit
  Larroque](https://github.com/zetaben))
* Optimize rendering performance by changing timing to fix
  textareas. [#941](haml/haml#941) (Takashi Kokubun)
* Fix `TypeError` with empty :ruby
  filter. [#942](haml/haml#942) (Takashi Kokubun)
* Fix inconsistent attribute sort order. (Takashi Kokubun)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment