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

SwissDict #634

Merged
merged 67 commits into from Aug 30, 2020
Merged

SwissDict #634

merged 67 commits into from Aug 30, 2020

Conversation

eulerkochy
Copy link
Member

@eulerkochy eulerkochy commented Jun 14, 2020

SwissDict

Part of GSoC'20 work.
Initial code and tests.
Tests run and works.
As of now, the benchmarks aren't favorable.
I'll post the benchmarks once I finish the work on #517

Tasks remaining

  • Refactor code, and improve benchmarks.
  • Add docstrings and deploy doc
  • Add file to README

@eulerkochy eulerkochy requested a review from oxinabox June 14, 2020 19:13
import Base: setindex!, sizehint!, empty!, isempty, length, copy, empty,
getindex, getkey, haskey, iterate, @propagate_inbounds, ValueIterator,
pop!, delete!, get, get!, isbitstype, in, hashindex, isbitsunion,
isiterable, dict_with_eltype, KeySet, Callable, _tablesz, filter!
Copy link
Member

Choose a reason for hiding this comment

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

Do we always do this per file?
I thought we just did this at the top level file

Copy link
Member Author

Choose a reason for hiding this comment

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

it's there in robin_dict.jl. Nice suggestion, let me get to it and try to get it out from there as well. I'll remove this from here as well.

copy(d::SwissDict) = SwissDict(d)
empty(d::SwissDict, ::Type{K}, ::Type{V}) where {K, V} = SwissDict{K, V}()

const AnyDict = SwissDict{Any,Any}
Copy link
Member

Choose a reason for hiding this comment

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

this is never used and seems unuseful and confusing

return dest
end

empty(a::AbstractDict, ::Type{K}, ::Type{V}) where {K, V} = SwissDict{K, V}()
Copy link
Member

Choose a reason for hiding this comment

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

Why are we chanign the default definition of empty for all AbstractDicts?
That doesn't seem right


#hashindex(key, sz) = (((hash(key)%Int) & (sz-1)) + 1)::Int

##Simd utilities
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
##Simd utilities
##SIMD utilities

Also fun, i haven't touched these llvmcall for SIMD in ages.

Comment on lines 117 to 132
@inline function _prefetchr(p::Ptr)
p = reinterpret(UInt, p)
if UInt === UInt64
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i64 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
else
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i32 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of the if by interpolating the type string in?

Suggested change
@inline function _prefetchr(p::Ptr)
p = reinterpret(UInt, p)
if UInt === UInt64
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i64 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
else
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i32 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
end
end
@inline function _prefetchr(p::Ptr)
p = reinterpret(UInt, p)
int_t = Int === Int64 ? "i64" : "int_t"
Core.Intrinsics.llvmcall(
("declare void @llvm.prefetch(i8*, i32, i32, i32)",
"""
%ptr = inttoptr $int_t %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""),
Nothing, Tuple{UInt}, p
)
end


@inline function _prefetchw(p::Ptr)
p = reinterpret(UInt, p)
if UInt === UInt64
Copy link
Member

Choose a reason for hiding this comment

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

similar to above, can we interpolate the type string in?

end

@propagate_inbounds function _slotget(slots::Vector{_u8x16}, i::Int)
#@boundscheck 0 < i <= length(slots)*16 || throw(BoundsError(slots, 1 + (i-1)>>4)
Copy link
Member

Choose a reason for hiding this comment

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

why is this commented out?

test/runtests.jl Outdated
"priority_queue",
"fenwick",
"priority_queue",
"fenwick",
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe you did not intend to add this white-space?

@rfourquet
Copy link
Contributor

Could you post a link to a reference on what are "Swiss dictionaries" ? It's difficult to find relevant results on search engines with these terms, also with "swiss map" ;-)

@eulerkochy
Copy link
Member Author

@rfourquet SwissDict as such doesn't exist. It is based on Abseil's flat_hash_map, which in turn uses Swiss Tables, develeped by Google engineers. Here's the cppcon talk by Matt Kulukundis which inspired my implementation.

@rfourquet
Copy link
Contributor

@eulerkochy Thanks!

Comment on lines 85 to 98
p = reinterpret(UInt, p)
if UInt === UInt64
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i64 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
else
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i32 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
p = reinterpret(UInt, p)
if UInt === UInt64
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i64 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
else
Core.Intrinsics.llvmcall(("declare void @llvm.prefetch(i8*, i32, i32, i32)", """
%ptr = inttoptr i32 %0 to i8*
call void @llvm.prefetch(i8* %ptr, i32 0, i32 3, i32 1)
ret void
"""), Nothing, Tuple{UInt}, p)
end
ccall("llvm.prefetch", llvmcall, Cvoid, (Ref{Int8}, Int32, Int32, Int32), Ptr{Int8}(p), 0, 3, 1)
end

@eulerkochy
Copy link
Member Author

eulerkochy commented Aug 29, 2020

K=Int64, 1e6 elements Dict SwissDict RobinDict
make 65.17 MB / 79.772 ms 22.66 MB / 70.462 ms 106.66 MB / 155.695 ms
pop! 134.000 ns 300.000 ns 325.00 ns
empty! 69.189 μs 39.550 μs 2.389 ms
find-success 60.000 ns 212.000 ns 179.000 ns
find-failure 230.000 ns 122.000 ns 142.000 ns

@eulerkochy eulerkochy changed the title [WIP] SwissDict SwissDict Aug 29, 2020
h.nbfull += (iszero(index & 0x0f) & (so==0x00))
_slotset!(h.slots, tag, index)
if index < h.idxfloor
h.idxfloor = index
Copy link
Member

Choose a reason for hiding this comment

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

do we need to fix the tests to make sure to cover this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't seem to find a test to cover this. It's quite hard to find a test case specifically for this. But, this serves the purpose of h.idxfloor, so it isn't wrong.

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.

None yet

8 participants