Improve type handling to work on 32-bit systems (Superseded by #20) #18
Conversation
| @@ -33,11 +33,11 @@ module Inflate | |||
| export inflate, inflate_zlib, inflate_gzip, | |||
| InflateStream, InflateZlibStream, InflateGzipStream | |||
|
|
|||
| # Huffman codes are internally represented by Vector{Vector{Int}}, | |||
| # Huffman codes are internally represented by Vector{Vector{UInt16}}, | |||
There was a problem hiding this comment.
Perhaps this should be done in a separate PR, but I think the tables of 64-bit elements are inefficient.
The bit width of this package is not very important because there are other performance bottlenecks in this package.
The important point here is that Unsigned should be used, not Signed.
There was a problem hiding this comment.
It would be useful to decouple 32-bit correctness from optimizations. The latter can be measured to verify if they are effective.
|
|
||
| abstract type AbstractInflateData end | ||
|
|
||
| mutable struct InflateData <: AbstractInflateData | ||
| bytes::Vector{UInt8} | ||
| current_byte::Int | ||
| current_byte::UInt8 |
There was a problem hiding this comment.
I beleve that *_byte should be a UInt8 type.
| if crc_table[1] == 0 | ||
| make_crc_table() | ||
| end |
There was a problem hiding this comment.
Isn't crc_table[1] always zero?
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #18 +/- ##
==========================================
- Coverage 99.35% 99.35% -0.01%
==========================================
Files 1 1
Lines 468 463 -5
==========================================
- Hits 465 460 -5
Misses 3 3 ☔ View full report in Codecov by Sentry. |
kimikage
changed the title
|
See #20. |
Fixes #16
I do not have a clear policy on the extent to which I should make changes. At least, this is not a minimal fix.