Add second patch to imcompress.c limiting cache to one tile#493
Add second patch to imcompress.c limiting cache to one tile#493esheldon wants to merge 1 commit into
Conversation
The old compressed tile cache could grow large. This limits it to size one tile, addressing the main performance gain from the cache.
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
I am not sure we should merge this fix.
We should be carrying patches that fix critical bugs etc. We should not carry patches for new features. The intent is that the patch is eventually merged upstream.
Downstream users should probably close and reopen their fits file on a regular basis maybe instead?
|
I think of this as an implementation detail not a feature. |
|
Right now this cfitsio "feature" is making it impossible to read DP2 healsparse maps in notebooks. I was going to just change to astropy fits reading because of it. Though I haven't checked what the overhead is of closing/opening. So I'll compare the speed/memory of astropy vs close/open vs this patch. |
|
OK. Even if you think this patch is an implementation detail, it needs to be combined with the existing patches etc. |
|
This patch works as is. Do you mean to do the other method instead, using a PR against cfitsio? |
|
Yes. The code here is more or less a hack of the build system. I refactored the patching code to make it clearer and easier to understand. |
|
OK, if we decide we want this I'll redo it. Let's see what Eli finds |
|
Superceded by #495 |
3 participants
The old compressed tile cache could grow large. This limits it to size one tile, addressing the main performance gain from the cache.
closes #492