Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for more than 65535 different attribute values #124

Merged
merged 1 commit into from Mar 10, 2018

Conversation

Projects
None yet
2 participants
@poire-z
Copy link
Contributor

poire-z commented Mar 10, 2018

Fix TOC and links with books with many footnotes, links and backlinks.
Attributes values (just like element names and attribute names) are strings stored in a table, and later referenced only by an integer value that points to this table entry.
These integer values were stored in a lUInt16 slot, limiting their number to 0-65535. When going above this limit, it started again at 0, overriding previous references, which messed attribute string retrieval (eg: 2nd book attribute was then mapped to the value set by the 65537th attribute).
So, id= and href= were wrong, impacting TOC entries and links.
We now store them in a lUint32 slot, allowing for a lot more different attribute values.

This fixes TOC and links on a french Bible (TOB), where every of the 30 000 verses are like:

 <a id='Luke_v13.4' href='#Luke_c13'>13.4</a>

with additional inter-verses links and links to glossary entries.
So, the 65535 limit was easily reached (it needs more than 90 000 slots).

Memory usage and cachefile sizes increase only by a small bit:
Before:

Opening big book load 24s+render 21s=153M
Page turn and back: 157M
Opening small book: 166M
Re-opening big book 3s=139M
Restarting koreader on this big book: 3s 109/112M
Final cache size for big/small book: 18 269 184 / 191 488

After:

Opening big book load 24s+render 21s=154M
Page turn and back: 158M
Opening small book: 167M
Re-opening big book 3s=143M
Restarting koreader on this big book: 3s 111/114M
Final cache size for big/small book: 18 551 808 / 192 512

I thought the compiler would have helped me a lot more with changing these types, but it did not.
Many segfaults till I found all the stuff needed change, and it works now on all kind of links I checked, with no crash. I'm not 100% sure I addressed every stuff, but well...
We can merge/bump this independantly in a few days.

Allow for more than 65535 different attribute values
Fix TOC and links with books with many footnotes, links and backlinks.
Attributes values (just like element names and attribute names) are strings
stored in a table, and later referenced only by an integer value that
points to this table entry.
These integer values were stored in a lUInt16 slot, limiting their
number to 0-65535. When going above this limit, it started again
at 0, overriding previous references, which messed attribute string
retrieval. So, id= and href= were wrong, impacting TOC entries and links.
We now store them in a lUint32 slot, allowing for a lot more different
attribute values.
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Mar 10, 2018

Sometimes clang can be more helpful than gcc.

@poire-z

This comment has been minimized.

Copy link
Contributor Author

poire-z commented Mar 10, 2018

Well, merging as I won't be doing any more tests on the emulator. I'll react if we notice bugs on our devices.

@poire-z poire-z merged commit fd27b10 into koreader:master Mar 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@poire-z poire-z deleted the poire-z:attrValues32bits branch Mar 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.