JuliaLang / AllocCheck.jl

AllocCheck
Other
209 stars 8 forks source link

Similar Package - TestNoAllocations.jl - Crossreference or Merging? #59

Open BloodWorkXGaming opened 8 months ago

BloodWorkXGaming commented 8 months ago

Hello everyone :) I have recently created a very similar Package called TestNoAllocations.jl which solves a similar purpose of finding allocations.

Considering the limitations of this package, AllocCheck.jl and TestNoAllocations.jl seem to complement each other pretty well.

Due to TestNoAllocations.jl only running during tests, there are no runtime costs at all, however, the guarantees of AllocCheck.jl are much stronger.

Is there any interest of a CrossReference between the two packages in the README or merging the package into one unified one?

baggepinnen commented 8 months ago

The method used by TestNoAllocations.jl has severe limitations, so severe in fact that the statement made in the readme is false

@testnoallocations which makes sure, that a tested implementation of a function is allocation free

julia> foo() = (rand() > 0.5 ? 1 : big"1") + 1
foo (generic function with 1 method)

julia> @warn_alloc foo()
2

According to above statement, I have now made sure that the function I tested is allocation free, yet:

julia> @warn_alloc foo()
┌ Warning: Detected 1 alocations (32 bytes) in foo(...)
│ ┌ Allocated Profile.Allocs.UnknownType (32 bytes) [1x]
│ ├─ foo @ ./REPL[7]:1
│ ├── + @ ./gmp.jl:555
│ ├─── + @ ./gmp.jl:549
│ ├──── add_ui @ ./gmp.jl:180
│ └───── BigInt @ ./gmp.jl:63
└ @ TestNoAllocations ~/.julia/packages/TestNoAllocations/nWmzM/src/alloc_profile.jl:66
2

In comparison, AllocCheck finds 3 possible allocations:

julia> using AllocCheck

julia> check_allocs(foo, ())
3-element Vector{Any}:
 Allocation of BigInt in ./gmp.jl:64
  | b = MPZ.init2!(new(), nbits)

Stacktrace:
 [1] _
   @ ./gmp.jl:64 [inlined]
 [2] BigInt
   @ ./gmp.jl:63 [inlined]
 [3] add_ui(a::BigInt, b::UInt64)
   @ Base.GMP.MPZ ./gmp.jl:180
 [4] +
   @ ./gmp.jl:549 [inlined]
 [5] +
   @ ./gmp.jl:555 [inlined]
 [6] foo()
   @ Main ./REPL[7]:1

 Allocating runtime call to "jl_get_ptls_states" in ./boot.jl:379
  | getptls() = ccall(:jl_get_ptls_states, Ptr{Cvoid}, ())

Stacktrace:
 [1] getptls
   @ ./boot.jl:379 [inlined]
 [2] finalizer
   @ ./gcutils.jl:93 [inlined]
 [3] _
   @ ./gmp.jl:65 [inlined]
 [4] BigInt
   @ ./gmp.jl:63 [inlined]
 [5] add_ui(a::BigInt, b::UInt64)
   @ Base.GMP.MPZ ./gmp.jl:180
 [6] +
   @ ./gmp.jl:549 [inlined]
 [7] +
   @ ./gmp.jl:555 [inlined]
 [8] foo()
   @ Main ./REPL[7]:1

 Allocating runtime call to "jl_gc_add_ptr_finalizer" in ./gcutils.jl:93
  | ccall(:jl_gc_add_ptr_finalizer, Cvoid, (Ptr{Cvoid}, Any, Ptr{Cvoid}),

Stacktrace:
 [1] finalizer
   @ ./gcutils.jl:93 [inlined]
 [2] _
   @ ./gmp.jl:65 [inlined]
 [3] BigInt
   @ ./gmp.jl:63 [inlined]
 [4] add_ui(a::BigInt, b::UInt64)
   @ Base.GMP.MPZ ./gmp.jl:180
 [5] +
   @ ./gmp.jl:549 [inlined]
 [6] +
   @ ./gmp.jl:555 [inlined]
 [7] foo()
   @ Main ./REPL[7]:1

What limitations of AllocCheck.jl does TestNoAllocations.jl not share?

BloodWorkXGaming commented 8 months ago

Yes, I wholeheartedly agree, that alloccheck has a way more sophisticated and accurate design.

The @warn_alloc macro is rather for finding allocations, not for testing for them in unit tests. That's what @testnoallocations is for, which is based on @allocated and shouldn't miss anything but can't determine the location.

Thanks for the note on check_allocs(foo....). From the readme I assumed every checked method had to be annotated and will have the allocating-limitation. Glad to see this isn't the case. I suggest adding this to the Limitations section in the readme.

The main difference between the two packages is checking statically (generally better) and checking actually happening allocations.

Will have to check whether this is the case, but one situation where I can see checking statically becoming problematic is when Boolean flags are being used. For example I often use bools to enable plotting for debugging purposes, which causes a lot of allocations. These plots could cause a ton of warnings which alloccheck but none with testnoallocations (on the disabled path).

baggepinnen commented 8 months ago

Have you seen https://julialang.github.io/AllocCheck.jl/dev/tutorials/optional_debugging_and_logging/ ?

baggepinnen commented 8 months ago

The @warn_alloc macro is rather for finding allocations, not for testing for them in unit tests. That's what @testnoallocations is for, which is based on @allocated and shouldn't miss anything but can't determine the location.

@testnoallocations has the same limitations:

julia> foo_test() = @testnoallocations foo()

julia> foo_test() # Test passed??
2

julia> foo_test() # Test failed this time
┌ Warning: Detected 1 alocations (32 bytes) in foo(...)
│ ┌ Allocated Profile.Allocs.UnknownType (32 bytes) [1x]
│ ├─ foo @ ./REPL[7]:1
│ ├── + @ ./gmp.jl:555
│ ├─── + @ ./gmp.jl:549
│ ├──── add_ui @ ./gmp.jl:180
│ └───── BigInt @ ./gmp.jl:63
└ @ TestNoAllocations ~/.julia/packages/TestNoAllocations/nWmzM/src/alloc_profile.jl:66
Test Failed at /home/fredrikb/.julia/packages/TestNoAllocations/nWmzM/src/allocation_macros.jl:47
  Expression: allocs === 0
   Evaluated: 32 === 0

ERROR: There was an error during testing
BloodWorkXGaming commented 8 months ago

Well, I should really read the docs more attentive! Sorry for that! that seems exactly what I was referring to.

BloodWorkXGaming commented 8 months ago

Will have to check what causes the situation you have described, as this shouldn't happen.

However once #46 or #54 will get merged testnoallocations will be redundant either way and will be superseded by alloccheck.

Thanks for your time and the discussion! :)