Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

memory handing and problems #79

Open
dpavlin opened this issue Mar 30, 2012 · 14 comments
Open

memory handing and problems #79

dpavlin opened this issue Mar 30, 2012 · 14 comments
Assignees

Comments

@dpavlin
Copy link
Member

dpavlin commented Mar 30, 2012

there are too many memory knobs as described by our problem to communicate correct settings on mobileread forum http://www.mobileread.com/forums/showpost.php?p=2023375&postcount=194

Right now, we in unireader.lua we have:

    -- tile cache configuration:
    cache_max_memsize = 1024*1024*5, -- 5MB tile cache
    cache_item_max_pixels = 1024*1024*2, -- max. size of rendered tiles

but we also have in pdfreader.lua:

function PDFReader:open(filename)
    -- muPDF manages its own cache, set second parameter
    -- to the maximum size you want it to grow
    local ok
    ok, self.doc = pcall(pdf.openDocument, filename, 64*1024*1024)

It seems to me that we need to do grep MemFree /proc/meminfo as sane default value (in lua, somehow :-) and then assign that in some ratio to cache_max_memsize, cache_item_max_pixels and mupdf if we are viewing pdf file (otherwise, it would be nice to have more memory for first two variables).

Any takers?

@houqp
Copy link
Member

houqp commented Mar 31, 2012

Are you referring to change the default value on the fly(before every document open) or just before kindlepdfview starts?

@dpavlin
Copy link
Member Author

dpavlin commented Mar 31, 2012

I think that getting free memory has to be done in lua, before first document is loaded, but re-distribution between buffers of that value should be done on each document open (for change between pdf and djvu #80 for example)

Even better, we can also check document size on open, and it's really massive ask user if they want to stop kindle framework to gain additional memory.

@houqp
Copy link
Member

houqp commented Mar 31, 2012

That will some how make the memory cache uncontrollable by users. So we need a very intelligent auto cache setting system if take this approach.

Also I notice that it is hard to predict memory usage according to file size. One of the file in our share point is only 3M, but still can leads to reboot. And another 100M pdf file of mine consumes less memory than that one. :(

@vdp
Copy link

vdp commented Mar 31, 2012

Here is an interesting observation in respect to the memory issues I am experiencing...
I increased "cache_max_memsize" to 15MB and "cache_item_max_pixels" to 5MB from their default values of 5MB and 2MB respectively. Then I open a file (the behavior is the same no matter whether it's a DJVU or PDF) and zoom in at "sufficiently" high magnification level. Then I cycle between two pages back and forth several times while watching the memory usage in 'top'. With these new settings for the tile cache the memory usage stays the same, whereas with the original/smaller values it raises steadily at a suspiciously uniform rate(say 2% per page turn).
I don't know enough to debug it myself, but my bet here is on the tile cache eviction/garbage collection code...

@houqp
Copy link
Member

houqp commented Mar 31, 2012

nice catch @vdp!

dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Mar 31, 2012
@houqp
Copy link
Member

houqp commented Mar 31, 2012

Just fixed a bug in pull request #84. The GC circle seems too long and before it do a collection, the size for biltbuffer userdata has already exceed the cache_max_memsize. (Actually, I have never seen a collection by watching output of collectgarbage("count")).

@dpavlin
Copy link
Member Author

dpavlin commented Mar 31, 2012

@vdp can you try https://github.com/downloads/dpavlin/kindlepdfviewer/kindlepdfviewer-b4bf2e5.zip

This includes @houqp fix #84 and memory usage on menu key. It seems to work nice in my limited testing on DXG.

@vdp
Copy link

vdp commented Apr 1, 2012

Thank you very much @houqp and @dpavlin !
I am using the repo HEAD now, which I assume is not much different from the zip which @dpavlin is referencing above.
For me the problem seems to be solved for all practical purposes. I mean I am not absolutely sure that there are no more leaks, but if there are any they are quite minor. I successfully turned about 150 or so pages in both DJVU and PDF. Because I tend to use the reader at high zoom levels I decided to increase the cache sizes somewhat and for the time being I will run kpdfview with the framework stopped(launchpad shortcuts). With cache_max_memsize set to 15MB and cache_document_size set to 20MB the highest memory usage that I observed with a DJVU was about 60% and for PDF it was below 40%(both for 'proper' and scanned PDF).
It's so much better when you don't have to worry that you are approaching 15 page turns and it is maybe better to restart the application :)

@houqp
Copy link
Member

houqp commented Apr 1, 2012

@vdp , we owe you a lot for the testing work and the interesting observation :)

Just one more note, seems that djvulibre and mupdf uses cachesize differently. Given the same cachesize, djvu will consume more memory. So the default cachse size for djvu is 10M, while mupdf is 64M. So if the memory usage is too high for you when reading djvu, you may want set two different document cache size for each format.

@vdp
Copy link

vdp commented Apr 1, 2012

Thanks, I will take this into account if I run into RAM-related issues with DJVU.

@dpavlin
Copy link
Member Author

dpavlin commented Apr 30, 2012

I managed to parse /proc/meminfo using code like

local mem_free = 8 * 1024 * 1024 -- 8Mb default

for line in io.lines("/proc/meminfo") do
        local kb = tonumber(line:match("MemFree: *([%d]+) kB"))
        if kb then mem_free = kb * 1024 end
end

print("Free memory "..mem_free.." bytes")

I think it would be save to use MemFree as start value and then divide it to cache_max_memsize and cache_document_size. My real question is what to do with settings variables? Should we just decrease them to value really available and treat them as maximums? Is 8/5 Mb split value we want to keep?

@houqp
Copy link
Member

houqp commented Apr 30, 2012

Here is my two cents.

I think we should respect users' choices, i.e. use cache_max_memsize and cache_document_size as the real value if specified by user in .reader.kpdfview.lua. That means user can have complete control over the cache system.

We can add a max_cache_max_memsize and max_cache_document_size in .reader.kpdfview.lua as the maximums limit if needed.

@dpavlin
Copy link
Member Author

dpavlin commented Apr 30, 2012

I would like to simplify current configuration, not to introduce two more configuration variables :-)

But we could decrease user supplied values if there isn't enough memory?

This seem like better solution that OOM killer which looks very scary to users, especially on DXG where memory is tight:

MemTotal:       127172 kB
MemFree:          3136 kB
Buffers:         10876 kB
Cached:          31156 kB

In fact, I might need to add in Cached to MemFree just to get some reasonable amount in DXG... And if it's under configured max_cache_max_memsize+max_cache_document_size+glyphcache_max_memsize we do nothing, if it's less than that, I would decrease values and return InfoMessage about it suggesting to restart reader without framework...

Seems reasonable?

@houqp
Copy link
Member

houqp commented May 1, 2012

Ok, that makes sense. I would suggest we try it out first :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants