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.

Sign up for GitHub

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

Fix #24: can the add() method return a boolean indicating a 'fresh' insert #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix #24: can the add() method return a boolean indicating a 'fresh' insert #27

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d7e1ac
Fix #24: add() method return a boolean indicating a 'fresh' insert
thomascharbonnel Mar 23, 2017
18a5077
Style: add blank line
thomascharbonnel Mar 23, 2017
44d66c8
Doc: add comment to BloomFilter#add
thomascharbonnel Mar 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions bloom-filter/src/main/scala/bloomfilter/mutable/BloomFilter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,27 @@ class BloomFilter[T] private (val numberOfBits: Long, val numberOfHashes: Int, p
this(numberOfBits, numberOfHashes, new UnsafeBitArray(numberOfBits))
}

/** Add an item in the Bloom filter
*
* @param x the T item to add
* @return true if the item was not in the filter before, false otherwise
*/
def add(x: T): Unit = {
Copy link
Owner

Choose a reason for hiding this comment

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

still return Unit

Copy link
Author

Choose a reason for hiding this comment

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

Ooops

val hash = canGenerateHash.generateHash(x)
val hash1 = hash >>> 32
val hash2 = (hash << 32) >> 32
var was_defined = true

var i = 0
while (i < numberOfHashes) {
val computedHash = hash1 + i * hash2
if (!bits.get((computedHash & Long.MaxValue) % numberOfBits))
Copy link
Owner

Choose a reason for hiding this comment

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

It's an additional read op that I really want to avoid. It can be done inside UnsafeBitArray but still.
What if it will be a separate method for AddAndGet semantic (needs better name for sure). WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

Why not for the separate method, addAndGet is not that bad (that's my way of saying that I can't think of anything clearer).
Do you think it's better to go the naive way

def addAndGet(x: T): Boolean = {
  val was_set = mightContain(x)
  add(x)
  !was_set
}

or C&P?

Copy link
Owner

Choose a reason for hiding this comment

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

I think the best way is a separate method in BloomFilter class and another one in UnsafeBitArray (it has the read operation there already)

UnsafeBitArray{
  def setAndGet?(index: Long): Boolean = {
    val offset = ptr + (index >>> 6) * 8L
    val long = unsafe.getLong(offset)
    if (bit not set in long) {
        unsafe.putLong(offset, long | (1L << index))
        true
    } else false
  }
}

was_defined = false
bits.set((computedHash & Long.MaxValue) % numberOfBits)
i += 1
}

!was_defined
}

def union(that: BloomFilter[T]): BloomFilter[T] = {
Expand All @@ -41,6 +51,7 @@ class BloomFilter[T] private (val numberOfBits: Long, val numberOfHashes: Int, p
val hash = canGenerateHash.generateHash(x)
val hash1 = hash >>> 32
val hash2 = (hash << 32) >> 32

var i = 0
while (i < numberOfHashes) {
val computedHash = hash1 + i * hash2
Expand Down