Skip to content
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

Fork updates #63

Merged
merged 3 commits into from
Dec 13, 2019
Merged

Fork updates #63

merged 3 commits into from
Dec 13, 2019

Conversation

voku
Copy link
Contributor

@voku voku commented Nov 23, 2019

  • support for more unicode chars
  • language-specific stop-words (disabled by default)
  • currency support
  • php7 types

This change is Reviewable

voku added 2 commits November 23, 2019 03:55
- support for more unicode chars
- language-specific stop-words (disabled by default)
- currency support
- php7 types
@lux
Copy link
Collaborator

lux commented Dec 10, 2019

I just had a chance to benchmark how these changes would affect the library's performance. Calling ::filter() 100k times had the following results:

Version Time
master before voku/portable-ascii merge 0.936257
master after voku/portable-ascii merge 5.796683
this branch 7.003144

I don't see this being a performance-critical piece of most apps, since once generated presumably the slugs would be stored somewhere, but I wanted to ask whether there were any options that can be tuned to speed things back up a bit, or if voku/portable-ascii provides any way to fall back to PHP extensions for parts of it when available?

@voku voku closed this Dec 13, 2019
@voku voku deleted the fork-updates branch December 13, 2019 00:18
@voku voku restored the fork-updates branch December 13, 2019 00:28
@voku
Copy link
Contributor Author

voku commented Dec 13, 2019

@lux I optimized the performance of the fork, if you have time, please check it. :)

-> https://github.com/voku/urlify

@lux
Copy link
Collaborator

lux commented Dec 13, 2019

Much faster! 1.768 from 5.7 or 7, nice :)

I see you closed the PR though. I didn't mean to imply I wasn't interested in merging your changes, just something I thought to look at to be thorough. Any chance of getting the performance improvements you made into this PR still? That would be awesome.

@voku voku reopened this Dec 13, 2019
@voku
Copy link
Contributor Author

voku commented Dec 13, 2019

@lux I back ported the changes, now we will easily call \voku\helper\ASCII::to_ascii() :)

@lux
Copy link
Collaborator

lux commented Dec 13, 2019

Thanks, much appreciated!

@lux lux merged commit b917c8c into jbroadway:master Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants