Add support for 32-bit systems #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 96.53% 96.56% +0.02%
==========================================
Files 5 5
Lines 231 233 +2
==========================================
+ Hits 223 225 +2
Misses 8 8
Continue to review full report at Codecov.
|
|
If JuliaLang/julia#35870 is fixed,, the tests on x86 may show the warning. However, I haven't been able to identify the reason why so many instruction pointers stored on 32-bit system. |
src/graph.jl
Outdated
| for child in values(node.down) | ||
| child.frame.from_c || continue | ||
| st |= status(child, C) | ||
| parents = [node] # use BFS (breadth first search) for simplicity |
There was a problem hiding this comment.
I'd say "for simplicity" seems misleading since this is quite a few more lines of code than the original. Is the stackoverflow a problem in practice?
There was a problem hiding this comment.
Yes, it means the simplicity compared to DFS and the original recursion is much simpler.
Is the stackoverflow a problem in practice?
Personally, the answer is no. Since flamegraphs are intended for visualization, too deep graphs are useless.
I don't know the cause, but on 32-bit systems, "TTFP" easily causes a stack overflow. For example, without Profile.init(n=10000), the test will fail.
It's no wonder that the 32-bit version is inefficient in some way and takes a longer process time, i.e. the width of the graph increases. However, it is strange that the depth increases.:confused:
As mentioned above, it would be meaningless to take measures only here, so I will revert it.
src/graph.jl
Outdated
| child.frame.from_c || continue | ||
| st |= status(child, C) | ||
| parents = [node] # use BFS (breadth first search) for simplicity | ||
| while !isempty(parents) || st === 0b111 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Thanks @kimikage! |
This fixes #28.
This also fixes the potential stack overflow ofstatus(). However,flamegraph!()can still cause a stack overflow. So, we can also remove the change from this PR.Profiling on 32-bit systems seems to be "somewhat" different than on 64-bit systems.