Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Improve the performance of filter and lines #332

Open
ndmitchell opened this issue Jun 11, 2017 · 6 comments
Open

Improve the performance of filter and lines #332

ndmitchell opened this issue Jun 11, 2017 · 6 comments
Labels
T - string Affect type Strings

Comments

@ndmitchell
Copy link
Contributor

I ported weeder to foundation. It goes 30% slower compared to [Char], and I was hoping for a significant speedup (that was the entire point of the port). The two most egregious things in the profile are:

39.2   Foundation.String.UTF8 lines (0)
30.8   Foundation.String.UTF8 filter (210)

Namely I spend 39% in lines, and because of #331 I need to add a filter that takes 30% of the time.

@vincenthz
Copy link
Member

at the moment there's no way to go faster because it's using the Prelude implementation of lines and filter. relatively easy fix though.

@vincenthz
Copy link
Member

could you try running it through conduit just to check for speed difference ?

Something like this should workaround the slow perf of String.lines:

import Foundation.Conduit.Textual (lines)

runConduit (sinkSource s .| lines .| sinkList)

@NicolasDP NicolasDP added enhancement T - string Affect type Strings labels Jun 11, 2017
@ndmitchell
Copy link
Contributor Author

Conduit goes way faster - it's now beating the [Char] version by 10% or so (which is still pretty crap - but does suggest the final result will be worth doing).

  • The revised benchmark shows lines taking only 5% (almost all of that in Textual.lines - suggesting that the conduit version might be reasonable as the final version...). Either way, copying the approach in Textual.lines is the right thing to do.
  • Filter now takes 48% of the runtime. I wonder if replace "\r" "" might work better? After Fixed #289 (String.replace function) #314 of course.
  • UTF8.fromBytes is now at 17%, so that's going to get to the bottleneck quite quickly.

@ndmitchell
Copy link
Contributor Author

For info, moving to basement means giving up on the conduit lines function I was using, making this function more important.

@vincenthz
Copy link
Member

vincenthz commented Jan 11, 2018

I think String.lines is now already much faster than it was and not using the compat Prelude.lines. it should really be similar in term of performance to conduit's lines since they are both using the same String.breakLine under the hood.

@ndmitchell
Copy link
Contributor Author

Then you can probably close this issue (although adding a benchmark might be interesting to see the overhead of conduit).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T - string Affect type Strings
Projects
None yet
Development

No branches or pull requests

3 participants