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

responsiveness improvements for connectbot #29

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@yulin2

yulin2 commented Feb 15, 2014

Hello developers of connectbot,

I'm a Ph.D. student and I'm doing research related to Android apps'
responsiveness. I found there are some cases in connectbot that the
database is accessed/queried by UI thread. Does this affect the
performance/responsiveness of the app? An optimization could be
extracting the DB operations into AsyncTask.

For example, in PubkeyListActivity.java, the "pubkeydb" is
queried/inserted/deleted at several places in UI thread: line 334,
408, 453, 468 and so on. How about put them into AsyncTask? I send a
sample patch here. Note that in the patch, in order to avoid data
races on "pubkeydb", I also add synchronization when UI thread and
AsyncTask tries to access "pubkeydb" (e.g., without synchronization,
races on "pubkeydb" appear between "UpdateListTask" and
"onStart"/"onStop").

Similarly, I also transformed HostListActivity.java and
HostEditorActivity.java for field "hostdb". TerminalManager.java
for "hostdb" and "pubkeydb".

Also, TerminalKeyListener.java invokes "bridge.transport.write" which
writes to IO. This also lead to a potential responsiveness issue. Thus, I
tried to move it into AsyncTask. In order to keep the original
semantics, I add a method "getResult()" in the AsyncTask. If the
written throws IOException or takes more than 5 seconds (i.e., the app
is not responsive anymore), "onKey" method will return false.

There should be some other places that can be improved. What do you think
about these improvements? My thought is we can improve the responsiveness if we try
to avoid the access to IO/Database in UI thread.

Thanks,
Yu

@yulin2

This comment has been minimized.

Show comment
Hide comment
@yulin2

yulin2 Mar 4, 2014

Hello developers of connectbot,

I sent these patches two weeks ago. Do you have any comments on them? Will such refactorings improve the responsiveness of connectbot?

Thanks,
Yu

yulin2 commented Mar 4, 2014

Hello developers of connectbot,

I sent these patches two weeks ago. Do you have any comments on them? Will such refactorings improve the responsiveness of connectbot?

Thanks,
Yu

@tornewuff

This comment has been minimized.

Show comment
Hide comment
@tornewuff

tornewuff Mar 30, 2014

Contributor

We're well over a year behind in looking at patches/bugs/etc and are currently working on fixing regressions for a new release, so don't expect too much. I haven't had a chance to look at this but yes, I am aware that the app does a few slow operations on the UI thread. I doubt it has any real effect on responsiveness, though; the UI is extremely simple.

Contributor

tornewuff commented Mar 30, 2014

We're well over a year behind in looking at patches/bugs/etc and are currently working on fixing regressions for a new release, so don't expect too much. I haven't had a chance to look at this but yes, I am aware that the app does a few slow operations on the UI thread. I doubt it has any real effect on responsiveness, though; the UI is extremely simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment