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

Add safe refs for no-op 'map' for List and Map #1455

Merged
merged 1 commit into from
May 17, 2018

Conversation

DanielMiddleton
Copy link
Contributor

This is a resubmission of #1189

This makes it so that the map methods on List and Map are "safe" and will return the original collection if no modifications were made. It relies on withMutations which appears to already do what is desired. I did do some performance testing this time, and the changes seem to actually improve the performance.

I wasn't sure the best benchmarks to use, so let me know of any particular cases I should look more closely at.

Here are the benchmarks I ran and their results:

describe('List', function() {
  describe('maps values', function() {
    var array1024 = [];
    for (var ii = 0; ii < 1024; ii++) {
      array1024[ii] = ii;
    }
    var list = Immutable.List(array1024);

    it('noops', function() {
      list.map(x => x);
    });

    it('squares', function() {
      list.map(x => x * x);
    });

    it('squares evens', function() {
      list.map(x => (x % 2 === 0 ? x * x : x));
    });

    it('squares first half', function() {
      list.map(x => (x < 512 ? x * x : x));
    });

    it('squares second half', function() {
      list.map(x => (x >= 512 ? x * x : x));
    });
  });
});
describe('Map', function() {
  describe('maps values', function() {
    var obj = {};

    for (var ii = 0; ii < 1024; ii++) {
      obj['x' + ii] = ii;
    }

    var map = Immutable.Map(obj);

    it('noops', function() {
      map.map(x => x);
    });

    it('squares', function() {
      map.map(x => x * x);
    });

    it('squares evens', function() {
      map.map(x => (x % 2 === 0 ? x * x : x));
    });

    it('squares first half', function() {
      map.map(x => (x < 512 ? x * x : x));
    });

    it('squares second half', function() {
      map.map(x => (x >= 512 ? x * x : x));
    });
  });
});
yarn run v1.3.2
$ node ./resources/bench.js
List > maps values noops
  Old:     8,781     9,079     9,398 ops/sec
  New:    15,475    15,568    15,661 ops/sec
  compare: -1 1
  diff: 71.46%
  rme: 2.43%
List > maps values squares
  Old:     8,602     8,865     9,143 ops/sec
  New:    10,565    10,632    10,700 ops/sec
  compare: -1 1
  diff: 19.94%
  rme: 2.2%
List > maps values squares evens
  Old:     8,535     8,847     9,182 ops/sec
  New:     9,000     9,310     9,642 ops/sec
  compare: -1 1
  diff: 5.23%
  rme: 3.54%
List > maps values squares first half
  Old:     8,709     8,990     9,289 ops/sec
  New:    12,350    12,450    12,552 ops/sec
  compare: -1 1
  diff: 38.48%
  rme: 2.34%
List > maps values squares second half
  Old:     8,828     9,141     9,478 ops/sec
  New:     9,482     9,761    10,058 ops/sec
  compare: 0 0
  diff: 6.77%
  rme: 3.26%
Map > maps values noops
  Old:     1,702     1,912     2,181 ops/sec
  New:     8,012     8,045     8,079 ops/sec
  compare: -1 1
  diff: 320.64%
  rme: 8.71%
Map > maps values squares
  Old:     1,444     1,519     1,602 ops/sec
  New:     3,030     3,053     3,076 ops/sec
  compare: -1 1
  diff: 100.97%
  rme: 3.7%
Map > maps values squares evens
  Old:     1,463     1,548     1,643 ops/sec
  New:     3,378     3,389     3,400 ops/sec
  compare: -1 1
  diff: 118.86%
  rme: 4.1%
Map > maps values squares first half
  Old:     1,485     1,563     1,649 ops/sec
  New:     3,724     3,812     3,903 ops/sec
  compare: -1 1
  diff: 143.79%
  rme: 4.04%
Map > maps values squares second half
  Old:     2,587     2,975     3,499 ops/sec
  New:     3,593     3,731     3,880 ops/sec
  compare: 1 -1
  diff: 25.4%
  rme: 10.93%
all done
Done in 109.94s.

* Implement map() in List and Map

Fixes immutable-js#679
@leebyron
Copy link
Collaborator

Thanks, this is a good step forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants