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 any function #34

Merged
merged 5 commits into from
Mar 8, 2024
Merged

Add any function #34

merged 5 commits into from
Mar 8, 2024

Conversation

flip111
Copy link
Contributor

@flip111 flip111 commented Feb 23, 2024

Hashmap implements Foldable.any but Foldable can not short-ciruit. For big hashmaps this should be faster if the value is match is found early.

I also had an alternative implementation but i think it's a bit slower (quick n dirty benchmark)

MapNode.prototype.any = function(pred, shift) {
  var skipmap = this.datamap | this.nodemap;
  while (skipmap !== 0) {
      var bit = lowestBit(skipmap);
      skipmap &= ~bit;

      if ((this.datamap & bit) !== 0) {
          var dataIndex = index(this.datamap, bit);
          if (pred(this.content[dataIndex * 2 + 1]))
              return true;
      } else {
          var nodeIndex = index(this.nodemap, bit);
          if (this.content[this.content.length - nodeIndex - 1].any(pred, shift + 5))
              return true;
      }
  }
  return false;
}

Copy link
Owner

@fehrenbach fehrenbach left a comment

Choose a reason for hiding this comment

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

Thanks!

Couple of things:

This seems to be missing code for the Collision nodes. We definitely need that.

Not sure whether the tests would have caught it. I suspect not. Tests on random data might have a hard time running into this. You'd need a collision and no earlier matching value. Shouldn't be too hard to construct a test case manually, though.

I think this can be slightly simplified, see below.

src/Data/HashMap.js Outdated Show resolved Hide resolved
@flip111
Copy link
Contributor Author

flip111 commented Feb 24, 2024

Thanks for the review.

and no earlier matching value

What do you mean with this?

src/Data/HashMap.js Outdated Show resolved Hide resolved
src/Data/HashMap.purs Show resolved Hide resolved
flip111 and others added 2 commits March 8, 2024 16:05
Co-authored-by: Stefan Fehrenbach <stefan.fehrenbach@gmail.com>
src/Data/HashMap.js Outdated Show resolved Hide resolved
src/Data/HashMap.purs Outdated Show resolved Hide resolved
Copy link
Owner

@fehrenbach fehrenbach left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@fehrenbach fehrenbach merged commit e8a0d0f into fehrenbach:master Mar 8, 2024
@flip111
Copy link
Contributor Author

flip111 commented Mar 8, 2024

Thank you too !

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