Skip to content

Tackle mcode_alloc issues once and for all#283

Merged
NiLuJe merged 100 commits into
koreader:masterfrom
NiLuJe:luajit-dlopen-mmap-what-fun
Jan 2, 2021
Merged

Tackle mcode_alloc issues once and for all#283
NiLuJe merged 100 commits into
koreader:masterfrom
NiLuJe:luajit-dlopen-mmap-what-fun

Conversation

@NiLuJe

@NiLuJe NiLuJe commented Dec 31, 2020

Copy link
Copy Markdown
Member

Okay, ignore the insane commit history, because this went through a lot of experimentation, especially since I found out my old Nexus 5 on 7.1 didn't give a damn about the initial workaround that yielded decent results on Android 10...

Anyway.

  • Move to a shared luajit library. This gets rid of the weird hack where the launcher was named libluajit for... reasons (related, ironically, to static linking, and old broken API levels).
  • Simplified the dlopen replacement given that fact, and actually implemented the system libs blacklist properly (not that it should be necessary).
  • And, as far as the LuaJIT mcode_alloc issues go:
  • Make sure the launcher requests a single 512K block right at startup.
  • Patch LuaJIT so that:
    • Flushing all traces doesn't unmap the mcode area, but instead zero it, so we don't "lose" our hard-earned mmap, given how tighly packed the virtual memory layout is on Android.
    • Reserve a 1MB area inside LuaJIT's address space via a global non-const, initialized array. This pretty much guarantees it'll be in the jumprange ;).
    • Make sure the very first alloc will punch a hole in that array, via MAP_FIXED

This change is Reviewable

Which implies renaming our main entry point ;).
But it works.
It works.
It's a plain array, and order is *very much* important here...
when I'm using the same thing in C without issue...

Weird-ass symbol shenanigans with multiple dlopen implementations?
But we're back to the random ffi.load failures (unless the JIT is
disabled).

The already loaded check doesn't make much sense, either.
@NiLuJe

NiLuJe commented Jan 1, 2021

Copy link
Copy Markdown
Member Author

@pazos: This iteration got rid of that ;). (I mean, the source is still there because I left the behavior optional for documentation purposes, but it's commented out and ifdef'ed out ;)).

@NiLuJe

NiLuJe commented Jan 1, 2021

Copy link
Copy Markdown
Member Author

But, yes, gradle would probably be the place to handle that properly. I just don't know how the hell this thing works ^^.

@pazos

pazos commented Jan 1, 2021

Copy link
Copy Markdown
Member

But, yes, gradle would probably be the place to handle that properly. I just don't know how the hell this thing works ^^.

Please merge this PR and I can take care of it next. I would merge it myself but I'm not sure if you want to keep the commit history or squash all commits together

@NiLuJe

NiLuJe commented Jan 1, 2021

Copy link
Copy Markdown
Member Author

Oh, definitely squash ;p.

Just have to double-check that the last commit didn't break everything ;p.

Spoiler: It did ;p. I've since fixed it, though :).

@NiLuJe NiLuJe merged commit af9b9d3 into koreader:master Jan 2, 2021
NiLuJe added a commit to koreader/koreader that referenced this pull request Jan 2, 2021
* Don't flag Android as should_restrict_JIT

And allow disabling the C blitter, to put the workaround to the test...

* Add a -d, --debug flag to the log function

Catches KOReader's debug, as well as (our) dlopen & luajit logging

* Bump android-luajit-launcher

koreader/android-luajit-launcher#283
koreader/android-luajit-launcher#282

* Bump base

(koreader/koreader-base#1279)
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Sep 28, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Oct 2, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Oct 3, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Oct 5, 2022
NiLuJe added a commit to koreader/koreader that referenced this pull request Oct 6, 2022
Basically:

* Use `extend` for class definitions
* Use `new` for object instantiations

That includes some minor code cleanups along the way:

* Updated `Widget`'s docs to make the semantics clearer.
* Removed `should_restrict_JIT` (it's been dead code since koreader/android-luajit-launcher#283)
* Minor refactoring of LuaSettings/LuaData/LuaDefaults/DocSettings to behave (mostly, they are instantiated via `open` instead of `new`) like everything else and handle inheritance properly (i.e., DocSettings is now a proper LuaSettings subclass).
* Default to `WidgetContainer` instead of `InputContainer` for stuff that doesn't actually setup key/gesture events.
* Ditto for explicit `*Listener` only classes, make sure they're based on `EventListener` instead of something uselessly fancier.
* Unless absolutely necessary, do not store references in class objects, ever; only values. Instead, always store references in instances, to avoid both sneaky inheritance issues, and sneaky GC pinning of stale references.
  * ReaderUI: Fix one such issue with its `active_widgets` array, with critical implications, as it essentially pinned *all* of ReaderUI's modules, including their reference to the `Document` instance (i.e., that was a big-ass leak).
* Terminal: Make sure the shell is killed on plugin teardown.
* InputText: Fix Home/End/Del physical keys to behave sensibly.
* InputContainer/WidgetContainer: If necessary, compute self.dimen at paintTo time (previously, only InputContainers did, which might have had something to do with random widgets unconcerned about input using it as a baseclass instead of WidgetContainer...).
* OverlapGroup: Compute self.dimen at *init* time, because for some reason it needs to do that, but do it directly in OverlapGroup instead of going through a weird WidgetContainer method that it was the sole user of.
* ReaderCropping: Under no circumstances should a Document instance member (here, self.bbox) risk being `nil`ed!
* Kobo: Minor code cleanups.
joshuacant pushed a commit to joshuacant/ProjectTitle that referenced this pull request Dec 22, 2024
Basically:

* Use `extend` for class definitions
* Use `new` for object instantiations

That includes some minor code cleanups along the way:

* Updated `Widget`'s docs to make the semantics clearer.
* Removed `should_restrict_JIT` (it's been dead code since koreader/android-luajit-launcher#283)
* Minor refactoring of LuaSettings/LuaData/LuaDefaults/DocSettings to behave (mostly, they are instantiated via `open` instead of `new`) like everything else and handle inheritance properly (i.e., DocSettings is now a proper LuaSettings subclass).
* Default to `WidgetContainer` instead of `InputContainer` for stuff that doesn't actually setup key/gesture events.
* Ditto for explicit `*Listener` only classes, make sure they're based on `EventListener` instead of something uselessly fancier.
* Unless absolutely necessary, do not store references in class objects, ever; only values. Instead, always store references in instances, to avoid both sneaky inheritance issues, and sneaky GC pinning of stale references.
  * ReaderUI: Fix one such issue with its `active_widgets` array, with critical implications, as it essentially pinned *all* of ReaderUI's modules, including their reference to the `Document` instance (i.e., that was a big-ass leak).
* Terminal: Make sure the shell is killed on plugin teardown.
* InputText: Fix Home/End/Del physical keys to behave sensibly.
* InputContainer/WidgetContainer: If necessary, compute self.dimen at paintTo time (previously, only InputContainers did, which might have had something to do with random widgets unconcerned about input using it as a baseclass instead of WidgetContainer...).
* OverlapGroup: Compute self.dimen at *init* time, because for some reason it needs to do that, but do it directly in OverlapGroup instead of going through a weird WidgetContainer method that it was the sole user of.
* ReaderCropping: Under no circumstances should a Document instance member (here, self.bbox) risk being `nil`ed!
* Kobo: Minor code cleanups.
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
* Don't flag Android as should_restrict_JIT

And allow disabling the C blitter, to put the workaround to the test...

* Add a -d, --debug flag to the log function

Catches KOReader's debug, as well as (our) dlopen & luajit logging

* Bump android-luajit-launcher

koreader/android-luajit-launcher#283
koreader/android-luajit-launcher#282

* Bump base

(koreader/koreader-base#1279)
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
Basically:

* Use `extend` for class definitions
* Use `new` for object instantiations

That includes some minor code cleanups along the way:

* Updated `Widget`'s docs to make the semantics clearer.
* Removed `should_restrict_JIT` (it's been dead code since koreader/android-luajit-launcher#283)
* Minor refactoring of LuaSettings/LuaData/LuaDefaults/DocSettings to behave (mostly, they are instantiated via `open` instead of `new`) like everything else and handle inheritance properly (i.e., DocSettings is now a proper LuaSettings subclass).
* Default to `WidgetContainer` instead of `InputContainer` for stuff that doesn't actually setup key/gesture events.
* Ditto for explicit `*Listener` only classes, make sure they're based on `EventListener` instead of something uselessly fancier.
* Unless absolutely necessary, do not store references in class objects, ever; only values. Instead, always store references in instances, to avoid both sneaky inheritance issues, and sneaky GC pinning of stale references.
  * ReaderUI: Fix one such issue with its `active_widgets` array, with critical implications, as it essentially pinned *all* of ReaderUI's modules, including their reference to the `Document` instance (i.e., that was a big-ass leak).
* Terminal: Make sure the shell is killed on plugin teardown.
* InputText: Fix Home/End/Del physical keys to behave sensibly.
* InputContainer/WidgetContainer: If necessary, compute self.dimen at paintTo time (previously, only InputContainers did, which might have had something to do with random widgets unconcerned about input using it as a baseclass instead of WidgetContainer...).
* OverlapGroup: Compute self.dimen at *init* time, because for some reason it needs to do that, but do it directly in OverlapGroup instead of going through a weird WidgetContainer method that it was the sole user of.
* ReaderCropping: Under no circumstances should a Document instance member (here, self.bbox) risk being `nil`ed!
* Kobo: Minor code cleanups.
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.

3 participants