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

Completely random table alloc tweaks in Lua/C modules #1350

Merged
merged 30 commits into from Apr 9, 2021

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Apr 8, 2021

Mainly pre-alloc when we can know/estimate the amount of elements, unify usage of raw setters for metatable-less tables, and stricter typing (e.g., don't cast integers to double by limiting usage of pushnumber to actual floats).

Also added some comments for the twistiest bits of logic (djvu, lookin' at ya').


This change is Reviewable

Copy link
Member Author

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @NiLuJe)

Copy link
Member Author

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @NiLuJe)

Copy link
Contributor

@poire-z poire-z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright after a quick skim of cre/xtext. Trusting you on the changes.

@NiLuJe
Copy link
Member Author

NiLuJe commented Apr 8, 2021

I've spent a bit of time doing weird things in the emu without breaking anything so far ;).

The wonkiest shit was very much in djvu, though, so I have no specific worries about xtext/cre.

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mainly visual to me besides the djvu thing… which we'll need to test a bit but I can't really say much about it.

We're clearing the stack, so, actually use that fact to simplify this ;).

(Because clearing the stack is otherwise entirely pedantic: what's
returned is always the uppermost n elements in the stack).
lua_objlen returns a size_t, and lua_tointeger a lua_Integer (usually a ptrdiff_t).
Copy link
Member Author

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @NiLuJe)

@NiLuJe NiLuJe merged commit 3163d9c into koreader:master Apr 9, 2021
roygbyte pushed a commit to roygbyte/koreader-base that referenced this pull request Mar 3, 2022
Mainly pre-alloc when we can know/estimate the amount of elements, unify usage of *raw* setters for metatable-less tables, and stricter typing (e.g., don't cast integers to double by limiting usage of `pushnumber` to actual floats).

Also added some comments for the twistiest bits of logic (djvu, lookin' at ya').
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants