Skip to content

Commit

Permalink
Replace murmur hash with djb2 hash (#203)
Browse files Browse the repository at this point in the history
* Replace murmur hash with djb2 hash

In profiling StyleSheet.create, I noticed that much of the time was
spent hashing. So, I found a faster hashing algorithm.

The implementation was taken from:

  https://github.com/darkskyapp/string-hash

According to this StackExchange post, this algorithm doesn't have as
good of randomness, but it has about the same percentage of collisions.
I don't think randomness matters for this application, so I think this
is okay.

  http://softwareengineering.stackexchange.com/a/145633

Using similar methodology to #202, this appears to make StylSheet.create
~15% faster (~220ms to ~185ms).

* Depend on string-hash for djb2 hashing algorithm

This is where I copied the code for this algorithm from, seems like we
might as well just bring in the dependency for it.
  • Loading branch information
lencioni authored and xymostech committed Mar 8, 2017
1 parent dbb3879 commit 21ef03b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 60 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@
},
"dependencies": {
"asap": "^2.0.3",
"inline-style-prefixer": "^2.0.0"
"inline-style-prefixer": "^2.0.0",
"string-hash": "^1.1.3"
},
"tonicExampleFilename": "examples/runkit.js"
}
52 changes: 2 additions & 50 deletions src/util.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* @flow */
import stringHash from 'string-hash';

/* ::
type Pair = [ string, any ];
Expand Down Expand Up @@ -155,55 +156,6 @@ export const stringifyValue = (
}
};

/**
* JS Implementation of MurmurHash2
*
* @author <a href="mailto:gary.court@gmail.com">Gary Court</a>
* @see http://github.com/garycourt/murmurhash-js
* @author <a href="mailto:aappleby@gmail.com">Austin Appleby</a>
* @see http://sites.google.com/site/murmurhash/
*
* @param {string} str ASCII only
* @return {string} Base 36 encoded hash result
*/
function murmurhash2_32_gc(str) {
let l = str.length;
let h = l;
let i = 0;
let k;

while (l >= 4) {
k = ((str.charCodeAt(i) & 0xff)) |
((str.charCodeAt(++i) & 0xff) << 8) |
((str.charCodeAt(++i) & 0xff) << 16) |
((str.charCodeAt(++i) & 0xff) << 24);

k = (((k & 0xffff) * 0x5bd1e995) + ((((k >>> 16) * 0x5bd1e995) & 0xffff) << 16));
k ^= k >>> 24;
k = (((k & 0xffff) * 0x5bd1e995) + ((((k >>> 16) * 0x5bd1e995) & 0xffff) << 16));

h = (((h & 0xffff) * 0x5bd1e995) + ((((h >>> 16) * 0x5bd1e995) & 0xffff) << 16)) ^ k;

l -= 4;
++i;
}

/* eslint-disable no-fallthrough */ // forgive existing code
switch (l) {
case 3: h ^= (str.charCodeAt(i + 2) & 0xff) << 16;
case 2: h ^= (str.charCodeAt(i + 1) & 0xff) << 8;
case 1: h ^= (str.charCodeAt(i) & 0xff);
h = (((h & 0xffff) * 0x5bd1e995) + ((((h >>> 16) * 0x5bd1e995) & 0xffff) << 16));
}
/* eslint-enable no-fallthrough */

h ^= h >>> 13;
h = (((h & 0xffff) * 0x5bd1e995) + ((((h >>> 16) * 0x5bd1e995) & 0xffff) << 16));
h ^= h >>> 15;

return (h >>> 0).toString(36);
}

// Hash a javascript object using JSON.stringify. This is very fast, about 3
// microseconds on my computer for a sample object:
// http://jsperf.com/test-hashfnv32a-hash/5
Expand All @@ -212,7 +164,7 @@ function murmurhash2_32_gc(str) {
// this to produce consistent hashes browsers need to have a consistent
// ordering of objects. Ben Alpert says that Facebook depends on this, so we
// can probably depend on this too.
export const hashObject = (object /* : ObjectMap */) /* : string */ => murmurhash2_32_gc(JSON.stringify(object));
export const hashObject = (object /* : ObjectMap */) /* : string */ => stringHash(JSON.stringify(object)).toString(36);


// Given a single style value string like the "b" from "a: b;", adds !important
Expand Down
2 changes: 1 addition & 1 deletion tests/index_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe('StyleSheet.create', () => {
},
});

assert.equal(sheet.test._name, 'test_y60qhp');
assert.equal(sheet.test._name, 'test_j5rvvh');
});

it('works for empty stylesheets and styles', () => {
Expand Down
16 changes: 8 additions & 8 deletions tests/inject_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ describe('String handlers', () => {
css(sheet.animate);
flushToStyleTag();

assertStylesInclude('@keyframes keyframe_1ptfkz1');
assertStylesInclude('@keyframes keyframe_1kmnkfo');
assertStylesInclude('from{left:10px;}');
assertStylesInclude('50%{left:20px;}');
assertStylesInclude('to{left:40px;}');
assertStylesInclude('animation-name:keyframe_1ptfkz1');
assertStylesInclude('animation-name:keyframe_1kmnkfo');
});

it('doesn\'t add the same keyframes twice', () => {
Expand Down Expand Up @@ -406,7 +406,7 @@ describe('String handlers', () => {
const styleTags = global.document.getElementsByTagName("style");
const styles = styleTags[0].textContent;

assert.include(styles, '@keyframes keyframe_1ptfkz1');
assert.include(styles, '@keyframes keyframe_1kmnkfo');
assert.equal(styles.match(/@keyframes/g).length, 1);
});

Expand Down Expand Up @@ -439,9 +439,9 @@ describe('String handlers', () => {
css(sheet.animate);
flushToStyleTag();

assertStylesInclude('@keyframes keyframe_1q5qq7q');
assertStylesInclude('@keyframes keyframe_1sbxkmr');
assertStylesInclude('animation-name:keyframe_1q5qq7q,keyframe_1sbxkmr')
assertStylesInclude('@keyframes keyframe_1a8sduu');
assertStylesInclude('@keyframes keyframe_1wnshbu');
assertStylesInclude('animation-name:keyframe_1a8sduu,keyframe_1wnshbu')
});

it('concatenates a custom keyframe animation with a plain string', () => {
Expand All @@ -464,8 +464,8 @@ describe('String handlers', () => {
css(sheet.animate);
flushToStyleTag();

assertStylesInclude('@keyframes keyframe_1q5qq7q');
assertStylesInclude('animation-name:keyframe_1q5qq7q,hoo')
assertStylesInclude('@keyframes keyframe_1a8sduu');
assertStylesInclude('animation-name:keyframe_1a8sduu,hoo')
});
});
});

2 comments on commit 21ef03b

@drKnoxy
Copy link

Choose a reason for hiding this comment

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

isn't this kind of a breaking change? as in, if you had unit tests they'd all fail

@xymostech
Copy link
Contributor

Choose a reason for hiding this comment

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

@drKnoxy I don't think we've typically counted the generated class names (nor the generated styles) as part of the API, so I'm not sure I'd count this as a breaking change. You're right that people might be depending on it, and we don't explicitly call that out. We should probably be clearer about what we count as our API.

Did we break tests for you?

Please sign in to comment.