Page MenuHomePhabricator

Optimize mw.text.trim() with default charset
Closed, ResolvedPublic

Description

mw.ustring.* methods are quite slow, and not needed when trimming with the default charset '\t\r\n\f '.

The implementation I'm submitting runs about 5x~10x times faster. Even the current implementation is fast, but this function is executed a lot of times, so numbers add up.

I'm using the variant 6 from http://lua-users.org/wiki/StringTrim. It is amongst the fastests, has no pitfall, and is a one-liner.

Event Timeline

Change 928693 had a related patch set uploaded (by Gerrit Patch Uploader; author: Anne Haunime):

[mediawiki/extensions/Scribunto@master] Optimize mw.text.trim() with default charset

https://gerrit.wikimedia.org/r/928693

In patchset 2, I'm replacing the default charset '\t\r\n\f ' with %s. This allows for a much simpler code.

The practical difference is that it adds \v (vertical tab) to the list of trimmed characters, which is also a good thing actually.

In patchset 3, I'm replacing s:match( ... ) with string.match( s, ... ).

The main reason is that if the input is not a string, or missing, the Lua error message is more helpful:

  • With mw.ustring.gsub(): « bad argument #1 to 'gsub' (string expected, got nil) »
  • With s:match(): « attempt to index a nil value »
  • With string.match(): « bad argument #1 to 'match' (string expected, got nil) »

Other reasons:

  • There are no uses of the s:method() syntax in the file currently.
  • It is a bit faster.

Also, I considered the variant 7 from http://lua-users.org/wiki/StringTrim but chose to not pick it. It is very slightly faster indeed, but I don't think it is worth it.

What is the license of the code you're copying? In general the detail you're commenting on this task is great, but please also add that information to the commit message :)

Added author and source in commit message of patchset 4. I can't say for the license. There is just none.

I have read the discussion on the mailing list, and David Manura is both the author of the http://lua-users.org/wiki/StringTrim compilation page, and of this specific code variant.

In commit message of patchset 5, added a note about the addition of \v to the characters trimmed by default.

Change 928693 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Optimize mw.text.trim() with default charset

https://gerrit.wikimedia.org/r/928693