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

Feature: array option #22

Merged
merged 4 commits into from
Jan 30, 2016
Merged

Feature: array option #22

merged 4 commits into from
Jan 30, 2016

Conversation

maranomynet
Copy link
Collaborator

(Adds to #21)

Add array option and move "joinResult" check into assemble

This adds a bit more flexibility and reduces overhead for the large majority of translations where no replacements are performed.

Removing the .bind() overhead from the default function probably doesn't hurt either.

@maranomynet maranomynet changed the title Feature: array option Feature: array option (#19) Jan 29, 2016
@maranomynet maranomynet changed the title Feature: array option (#19) Feature: array option Jan 29, 2016
This adds flexibility and reduces overhead for the
large majority of translations where no replacements
are performed.
@maranomynet
Copy link
Collaborator Author

Prelminary benchmarking on Google Chrome (locally) suggests that simply switching from Array#concat to Array#slice does indeed speed things up quite a bit. (I'm not sure how that rhymes with this performance discusssion but concat might be slower for small arrays)

But, regardless, this does not give the ~10x speed boost I'm looking for. :-(

@maranomynet
Copy link
Collaborator Author

Switching from Array#slice to starting with a new array and filling it in the loop (i.e. result[i+1] = parts[i+1]) gives an even further speed boost.
Still nowhere near enough.

By approximately...

 * 8x for `t(key, repl)`
 * 3x for `t.arr(key, repl)`
function assemble (parts, replacements, count, debug) {
var result = [].concat(parts)
function assemble (parts, replacements, count, debug, array) {
var result = array ? [parts[0]] : parts[0]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is odd... maybe asArray

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.

what about the options property-name? suggestions about that?

This seems to dent performance by 50% on Chrome,
but boosts performance by 450% on Firefox.
@maranomynet
Copy link
Collaborator Author

Please reveiew these changes and merge them if they look good to you.

I don't plan to do any more changes for now.

@StephanHoyer
Copy link
Owner

will look to it tomorrow

StephanHoyer added a commit that referenced this pull request Jan 30, 2016
@StephanHoyer StephanHoyer merged commit 345857f into master Jan 30, 2016
@StephanHoyer StephanHoyer deleted the feature/array_option branch January 30, 2016 23:18
@StephanHoyer
Copy link
Owner

merged and released under 0.4.1

maybe we should do an 1.0 any time soon

@maranomynet maranomynet restored the feature/array_option branch January 30, 2016 23:30
@StephanHoyer StephanHoyer deleted the feature/array_option branch May 11, 2016 09:44
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