#297 ✓resolved
Phred

String#camelize performance optimization

Reported by Phred | August 20th, 2008 @ 11:51 PM | in 1.7

String#camelize is insanely long for something that can be completed in one line. In addition, the replacement is much faster.

Code

Before:


function() {
  var parts = this.split('-'), len = parts.length;
  if (len == 1) return parts[0];

  var camelized = this.charAt(0) == '-'
    ? parts[0].charAt(0).toUpperCase() + parts[0].substring(1)
    : parts[0];

  for (var i = 1; i < len; i++)
    camelized += parts[i].charAt(0).toUpperCase() + parts[i].substring(1);

  return camelized;
}

After:


function() {
  return this.replace(/\-+(.)?/g, function(match, char) { return (char || '').toUpperCase() });
}

Performance

Browser: old way timing/new way timing (all times in ms)

Safari 3.1.2 (windows): 1484/391 FF 3.0.1: 1352/755 IE6: 7031/1891 Opera 9.52: 4406/2375

Performance Summary

almost 4x faster in Safari and IE6 almost 2x in FF3 and Opera

All timings taken from http://files.myopera.com/fearpha... (click "download file" if prompted). Be patient, it takes a bit to run.

The last time I suggested this fix, someone mentioned version of safari not supporting a function as the 2nd param to replace. A brief google search found articles as early as 2005 saying this was resolved then so that should no longer be an issue.

Comments and changes to this ticket

  • Phred

    Phred August 20th, 2008 @ 11:53 PM

    Legible performance numbers * Safari 3.1.2 (windows) - 1484 / 391 * FF 3.0.1 - 1352 / 755 * IE6 - 7031 / 1891 * Opera 9.52 - 4406 / 2375

  • John-David Dalton

    John-David Dalton August 21st, 2008 @ 12:15 AM

    +10 thank you for posting benchmarks

    Safari 2.0 has an issue with passing a function as the second param. It will try to convert it to a string.

    We could do

    
    
    String.prototype.camelize = (function() {
    
      'a'.replace('a', function() { return 'b' }) === 'b' ?
        return function() {
          this.replace(/\-+(.)?/g, function(match, char) {
            return (char || '').toUpperCase();
          })
        } :
        return function() {
          var parts = this.split('-'), len = parts.length;
          if (len == 1) return parts[0];
    
          var camelized = this.charAt(0) == '-'
            ? parts[0].charAt(0).toUpperCase() + parts[0].substring(1)
            : parts[0];
    
          for (var i = 1; i < len; i++)
            camelized += parts[i].charAt(0).toUpperCase() + parts[i].substring(1);
    
          return camelized;
        };
    })();
    
  • John-David Dalton

    John-David Dalton August 21st, 2008 @ 12:14 AM

    • Milestone set to 1.7
    • State changed from “new” to “enhancement”
    • Tag changed from enhancement, performance, string to benchmarked, enhancement, needs_patch, needs_tests, performance, string
  • Juriy Zaytsev

    Juriy Zaytsev August 21st, 2008 @ 05:49 AM

    Safari 2 should be killed violently ; )

    All the workarounds for a lack of hasOwnProperty and function-based replace are becoming ridiculous.

  • John-David Dalton

    John-David Dalton August 21st, 2008 @ 06:37 AM

    Ya I am starting to think the same thing.

  • Phred

    Phred August 21st, 2008 @ 09:47 AM

    For performance reasons, I would suggest that checked it once (when defining camelize) and not every single time it was used.

    Alternatively, you could sniff safari 2.x and fix String#replace:

    
    (function(replace){
      String.prototype.replace = function(pattern, replacement){
        // replacement is not function, use built-in
        if (typeof replacement != 'function')
          return replace.apply(this, arguments);
    
        var str = '' + this;
        // pattern string is not RegExp
        if (!(pattern instanceof RegExp)) {
          var idx = str.indexOf(pattern);
          return (idx == -1 ?
            str :
            replace.apply(str, [pattern, replacement(pattern, idx, str)]);
          );
        }
    
        var reg = pattern, result = [], lastidx = reg.lastIndex, re;
        while ((re = reg.exec(str)) != null) {
          var idx  = re.index, args = re.concat(idx, str);
          result.push(str.slice(lastidx, idx), replacement.apply(null, args).toString());
          if (!reg.global) {
            lastidx += RegExp.lastMatch.length;
            break;
          }
          else {
            lastidx = reg.lastIndex;
          }
        }
        result.push(str.slice(lastidx));
        return result.join('');
      }
    })(String.prototype.replace);
    

    This is old code (sanitized by me) that I found here.

  • Tobie Langel

    Tobie Langel August 22nd, 2008 @ 01:33 AM

    Premise:

    String#camelize is insanely long for something that can be completed in one line

    Initial code:

    
    function() {
      var parts = this.split('-'), len = parts.length;
      if (len == 1) return parts[0];
    
      var camelized = this.charAt(0) == '-'
        ? parts[0].charAt(0).toUpperCase() + parts[0].substring(1)
        : parts[0];
    
      for (var i = 1; i < len; i++)
        camelized += parts[i].charAt(0).toUpperCase() + parts[i].substring(1);
    
      return camelized;
    }
    
    

    What we end up with:

    
    (function(replace){
      String.prototype.replace = function(pattern, replacement) {
        // replacement is not function, use built-in
        if (typeof replacement != 'function')
          return replace.apply(this, arguments);
    
        var str = '' + this;
        // pattern string is not RegExp
        if (!(pattern instanceof RegExp)) {
          var idx = str.indexOf(pattern);
          return (idx == -1 ?
            str :
            replace.apply(str, [pattern, replacement(pattern, idx, str)]);
          );
        }
    
        var reg = pattern, result = [], lastidx = reg.lastIndex, re;
        while ((re = reg.exec(str)) != null) {
          var idx  = re.index, args = re.concat(idx, str);
          result.push(str.slice(lastidx, idx), replacement.apply(null, args).toString());
          if (!reg.global) {
            lastidx += RegExp.lastMatch.length;
            break;
          }
          else {
            lastidx = reg.lastIndex;
          }
        }
        result.push(str.slice(lastidx));
        return result.join('');
      }
    })(String.prototype.replace);
    
    function() {
      return this.replace(/\-+(.)?/g, function(match, char) { return (char || '').toUpperCase() });
    }
    

    Am I the only one to feel weird about this ?

  • Tobie Langel

    Tobie Langel August 22nd, 2008 @ 01:37 AM

    • Tag changed from benchmarked, enhancement, needs_patch, needs_tests, performance, string to needs_benchmarks, needs_patch, needs_tests, performance, string

    More seriously though, this is an optimization that depends on a new feature (fixing String#replace for Safari). It should be marked as such and a new ticket should be opened for String#replace fix. Also benchmarks should be made available (i.e. provided using the benchmark method in the unit test suite and commented out not to impact the speed of the tests.

  • Juriy Zaytsev

    Juriy Zaytsev August 22nd, 2008 @ 03:05 AM

    Tobie,

    I would like to propose dropping support for Safari 2.0.0 - 2.0.2. Those 3 minor releases bring havoc upon us.

    The #camelize situation shows how bizarre things are (and how much better they can be).

    Can we consider this for 1.6.1?

  • Phred

    Phred August 22nd, 2008 @ 07:02 AM

    Am I the only one to feel weird about this ? The difference being that the old camelize can be accomplished with much more concise code. Fixing replace is not as simplistic.

    Also benchmarks should be made available (i.e. provided using the benchmark method in the unit test suite and commented out not to impact the speed of the tests. It is very hard to find someone with Safari 2. I asked around to see if I could acquire some benchmarks. Would it be wrong for users of old stuff to take a performance hit?

    I would like to propose dropping support for Safari 2.0.0 - 2.0.2. This seems like a reasonable option as well.

    Do any of you know anyone with access to Safari 2? I think benchmarking Safari 2 will be the most difficult part of this fix.

  • Phred

    Phred August 22nd, 2008 @ 07:24 AM

    Is there a place where browser support is listed? Google and I can not find it.

    Side note: I hate the new wiki formatting stuff and it sucks not being able to edit comments.

  • John-David Dalton

    John-David Dalton August 22nd, 2008 @ 07:43 AM

    I have Safari 2.0 native but you can also download it here: http://michelf.com/projects/multi-safari/

    If you produce the benchmarks I can test it. I think its simpler to drop support for it and release an optional adapter if a dev requests support. I believe Apple even dropped support for 2.0

  • Phred

    Phred August 22nd, 2008 @ 08:10 AM

    Supported versions (for reference):

    • jQuery - Safari 2.0.2+
    • YUI - Safari 3.1+
    • Rico - Safari 2.0.3
    • Dojo - Latest Safari (currently 3.0.x), 2.0 support was dropped upon the release of Leopard
    • Ext JS - Safari 3+
    • qooxdoo - Safari 3.0
    • GWT - Safari 2.0
  • Phred

    Phred August 22nd, 2008 @ 08:15 AM

    more...

    Mootools - Safari 3+ MochiKit - 2.0.2

  • Tobie Langel

    Tobie Langel August 22nd, 2008 @ 10:16 AM

    I'm not too concerned about performance issues in Safari < 3. However, we should still aim to support it as much as possible, or at least, not break it on purpose.

    I'd definitely opt for modifying String#replace where necessary.

    I'd avoid the instanceof Regexp part, though, in favor of a duck-typing to avoid cross frame issues.

  • Phred

    Phred August 22nd, 2008 @ 11:17 AM

    Filed String#replace for Safari 2 as bug #300

    I didn't incorporate the duck typing.

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:26 AM

    • Tag changed from needs_benchmarks, needs_patch, needs_tests, performance, string to missing:tests, needs_benchmarks, needs_patch, performance

    [not-tagged:"needs_tests" tagged:"missing:tests" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:27 AM

    • Tag changed from missing:tests, needs_benchmarks, needs_patch, performance to missing:benchmarks, missing:tests, needs_patch, performance

    [not-tagged:"needs_benchmarks" tagged:"missing:benchmarks" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:28 AM

    • Tag changed from missing:benchmarks, missing:tests, needs_patch, performance to missing:benchmarks, missing:patch, missing:tests, performance

    [not-tagged:"needs_patch" tagged:"missing:patch" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:36 AM

    • Tag changed from missing:benchmarks, missing:patch, missing:tests, performance to missing:benchmarks, missing:patch, needs:tests, performance

    [not-tagged:"missing:tests" tagged:"needs:tests" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:37 AM

    • Tag changed from missing:benchmarks, missing:patch, needs:tests, performance to missing:benchmarks, needs:patch, needs:tests, performance

    [not-tagged:"missing:patch" tagged:"needs:patch" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:39 AM

    • Tag changed from missing:benchmarks, needs:patch, needs:tests, performance to needs:benchmarks, needs:patch, needs:tests, performance

    [not-tagged:"missing:benchmarks" tagged:"needs:benchmarks" bulk edit command]

  • Samuel Lebeau

    Samuel Lebeau August 26th, 2009 @ 08:08 AM

    • Assigned user set to “Samuel Lebeau”
  • Samuel Lebeau

    Samuel Lebeau September 3rd, 2009 @ 09:59 PM

    • Tag changed from needs:benchmarks, needs:patch, needs:tests, performance to needs:benchmarks, patched, performance
  • Yaffle

    Yaffle September 25th, 2009 @ 06:26 PM

    +1 for

      function camelize() {
        return this.replace(/\-+(\S)?/g, function(match, chr) {
          return chr ? chr.toUpperCase() : '';
        });
      }
    
  • GitHub Robot

    GitHub Robot September 26th, 2009 @ 07:42 PM

    • State changed from “enhancement” to “resolved”

    (from [f4ea4c6ef7f48dca963e54fc6203350b2d233b97]) Rewrite String#camelize using String#replace with a replacement function [#297 state:resolved] http://github.com/sstephenson/prototype/commit/f4ea4c6ef7f48dca963e...

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Referenced by

Pages