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 pool sharing with copy on write #56

Merged
merged 39 commits into from
Mar 1, 2021
Merged
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
96e5d3a
add pool sharing to PooledArrays
bkamins Feb 20, 2021
dd77169
further code review
bkamins Feb 20, 2021
7932121
use Atomic{Int} and implement copyto!
bkamins Feb 20, 2021
8dbee32
fix copy
bkamins Feb 20, 2021
92850e5
fix typo
bkamins Feb 20, 2021
995b3cd
fix leftover code
bkamins Feb 20, 2021
4c407f3
add missing )
bkamins Feb 20, 2021
00af820
fix small issues
bkamins Feb 20, 2021
45a33a9
add missing method
bkamins Feb 20, 2021
da13879
another missing method
bkamins Feb 20, 2021
665bf9a
fix various lurking problems (and bugs) in old code
bkamins Feb 20, 2021
cb1308c
Apply suggestions from code review
bkamins Feb 21, 2021
be31412
apply comments from the review
bkamins Feb 21, 2021
b41e9a1
improve getindex
bkamins Feb 21, 2021
07e7b9c
add view
bkamins Feb 21, 2021
1419283
Apply suggestions from code review
bkamins Feb 21, 2021
a1df0a0
Merge remote-tracking branch 'origin/main' into bk/pool_sharing
bkamins Feb 21, 2021
0cb0021
add SubArray handling
bkamins Feb 21, 2021
be09d78
Apply suggestions from code review
bkamins Feb 23, 2021
7f5f7e2
Update src/PooledArrays.jl
bkamins Feb 23, 2021
61bcbdc
add PooledArrOrSub
bkamins Feb 24, 2021
8ec3854
start adding tests
bkamins Feb 24, 2021
cd20319
Apply suggestions from code review
bkamins Feb 24, 2021
ee6100b
Merge remote-tracking branch 'origin/bk/pool_sharing' into bk/pool_sh…
bkamins Feb 24, 2021
02b002b
merge methods (currently fails but I first need to understand if it i…
bkamins Feb 24, 2021
21b3354
enough to remove where
bkamins Feb 24, 2021
ec0f710
simplify definition
bkamins Feb 24, 2021
35430e0
update getindex
bkamins Feb 24, 2021
d3fb0a3
continue adding tests
bkamins Feb 24, 2021
b6e6c85
fix current tests
bkamins Feb 24, 2021
2af32ca
finalize tests
bkamins Feb 24, 2021
c0333af
hopefully final fixes
bkamins Feb 24, 2021
46d971c
add Julia 1.0.5 support
bkamins Feb 24, 2021
c55b8e9
Update src/PooledArrays.jl
bkamins Feb 25, 2021
03a3221
fixes after code review
bkamins Feb 27, 2021
0b8ba4d
Merge remote-tracking branch 'origin/bk/pool_sharing' into bk/pool_sh…
bkamins Feb 27, 2021
3a92892
Update src/PooledArrays.jl
bkamins Feb 27, 2021
49afb14
Merge remote-tracking branch 'origin/bk/pool_sharing' into bk/pool_sh…
bkamins Feb 27, 2021
3eb65e7
Update src/PooledArrays.jl
bkamins Feb 28, 2021
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
Prev Previous commit
Next Next commit
fix current tests
  • Loading branch information
bkamins committed Feb 24, 2021
commit b6e6c853f08403ba8afeb2fb396921481959a424
15 changes: 7 additions & 8 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -232,32 +232,31 @@ end
@test pat1 == pa1
@test DataAPI.refpool(pat1) == DataAPI.refpool(pa1)
@test DataAPI.invrefpool(pat1) == DataAPI.invrefpool(pa1)
@test refcount(pat1) == refcount(pa1)
@test refcount(pat1) === refcount(pa1)
@test refcount(pat1)[] == 3

copy!(pat1, pav1)
@test pat1 == pav1
Copy link
Member Author

Choose a reason for hiding this comment

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

@nalimilan - this line fails without the special method for Julia 1.0:

julia> pa = PooledArray(1:4)
4-element PooledArray{Int64,UInt32,1,Array{UInt32,1}}:
 1
 2
 3
 4

julia> pav1 = @view pa[2:3]
2-element view(::PooledArray{Int64,UInt32,1,Array{UInt32,1}}, 2:3) with eltype Int64:
 2
 3

julia> pav1[1]
ERROR: MethodError: getindex(::SubArray{Int64,1,PooledArray{Int64,UInt32,1,Array{UInt32,1}},Tuple{UnitRange{Int64}},true}, ::Int64) is ambiguous. Candidates:
  getindex(A::SubArray{#s22,N,#s20,I,L} where L where I where #s20<:PooledArray where #s22, I::Vararg{Int64,N}) where {T, N} in PooledArrays at /home/bkamins/.julia/dev/PooledArrays/src/PooledArrays.jl:476
  getindex(V::SubArray{T,N,P,I,true} where I<:Tuple{AbstractUnitRange,Vararg{Any,N} where N} where P where N where T, i::Int64) in Base at subarray.jl:234
Possible fix, define
  getindex(::SubArray{T,1,P,I,true} where I<:Tuple{AbstractUnitRange,Vararg{Any,N} where N} where P<:PooledArray where T, ::Int64)
Stacktrace:
 [1] top-level scope at none:0

@test DataAPI.refpool(pat1) == DataAPI.refpool(pav1)
@test DataAPI.invrefpool(pat1) == DataAPI.invrefpool(pav1)
@test refcount(pat1) == refcount(pav1)
@test refcount(pat1) === refcount(pav1)
@test refcount(pat1)[] == 3

pat2 = PooledArray(fill(0))
copy!(pat2, pa2)
@test pat2 == pa2
@test DataAPI.refpool(pat2) == DataAPI.refpool(pa2)
@test DataAPI.invrefpool(pat2) == DataAPI.invrefpool(pa2)
@test refcount(pat2) == refcount(pa2)
@test refcount(pat2) === refcount(pa2)
@test refcount(pat2)[] == 2

copy!(pat2, pav1)
copy!(pat2, pav2)
@test pat2 == pav2
@test DataAPI.refpool(pat2) == DataAPI.refpool(pav2)
@test DataAPI.invrefpool(pat2) == DataAPI.invrefpool(pav2)
@test refcount(pat2) == refcount(pav2)
@test refcount(pat2)[] == 1
@test refcount(pa)[] == 4

@test refcount(pat2) === refcount(pav2)
@test refcount(pat2)[] == 4
@test refcount(pa2)[] == 1
end

@testset "correct refcount when treading" begin
Expand Down