Skip to content

Optimize escape functions#188

Open
isaacl wants to merge 1 commit into
playframework:mainfrom
isaacl:sbOpt
Open

Optimize escape functions#188
isaacl wants to merge 1 commit into
playframework:mainfrom
isaacl:sbOpt

Conversation

@isaacl

@isaacl isaacl commented Aug 20, 2018

Copy link
Copy Markdown
  • Use java.lang.StringBuilder instead of scala wrapper
  • Rewrite Html.buildString to use bulk copy methods
    when possible.

@lightbend-cla-validator

Copy link
Copy Markdown

Hi @isaacl,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@isaacl

isaacl commented Aug 20, 2018

Copy link
Copy Markdown
Author

cla signed

- Use java.lang.StringBuilder instead of scala wrapper
- Rewrite Html.buildString to use bulk copy methods
  when possible.
@isaacl

isaacl commented Aug 25, 2018

Copy link
Copy Markdown
Author

ping... I also posted in chat and got no response....

@marcospereira

Copy link
Copy Markdown
Member

Hi @isaacl,

I think it would be good to have JHM benchmarks added to the project so that we can better see/prove/check performance numbers when doing such optimizations:

https://github.com/ktoso/sbt-jmh

Basically, you write a benchmark for the function and run it before and after the optimization to see how the code change affected performance.

Do you have the time to do it?

Best.

@isaacl

isaacl commented Aug 27, 2018

Copy link
Copy Markdown
Author

if you're willing to do it, that would be easier for me, but I will get to it eventually.

@mkurz

mkurz commented Nov 10, 2018

Copy link
Copy Markdown
Member

@isaacl Do you have time to add the benchmarks?

@isaacl

isaacl commented Feb 28, 2019

Copy link
Copy Markdown
Author

Do you have a benchmark framework to use or are you asking for simple jmh?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants