Office Hours — Today, November 30

Yesterday, November 29

Nov 30
3:50 PM
Mark M.
has entered the room
3:55 PM
Mark M.
turned on guest access
4:00 PM
Steve
has entered the room
Steve
Hey Mark!
Mark M.
hello, Steve!
how can I help you today?
Steve
I actually saw you at the droid con in NYC :) Nice talk
Mark M.
thanks!
Steve
I am having an issue with AsyncTasks
View paste
We use custom AsyncTasks for networking operations that will add extra data/params when calling our endpoints. In most cases, we use them normally by calling Task.execute() to trigger doInBackground() and onPostExecute().

However, in an exceptional case, we want the AsyncTask to block so that we can wait for the return value of the network call. In this case we use get() instead of execute() while running on a worker thread.

However- when using get() instead of execute(), the call just throws a TimeoutException, and it doesn’t seem that doInBackground() is called at all. As it turns out, I ALWAYS get this TimeoutException unless I actually call Task.execute().get()
(via https://stackoverflow.com/a/10972142/5301868)

My problem- because this is a blocking call, I need to run on a worker thread (so I am running on an annotated @WorkerThread method), but because I need to call execute() I am getting warnings that execute() needs to be called from the main thread.
Mark M.
isn't everyone? :-)
Steve
Lol :P
Is this actually a problem? Am I doing something wrong by having to call execute() before get()?
Mark M.
OK, let's take this from the end and work backwards
there is no point in having this on a worker thread in your current implementation
as get() is a blocking call, and so it ties up whatever thread you're on
and I assume that's the main application thread.
Steve
Right
Mark M.
you would get the same basic results by extracting the code from doInBackground() and calling it directly
Steve
Currently this is all happening within an IntentService, on a worker thread
Mark M.
wait
you're having an IntentService execute an AsyncTask?
if so, why? you're already on a background thread; another background thread won't make the work background-er
4:05 PM
Steve
Yeah, but we essentially do not need this AsyncTask to create a new thread.
4:05 PM
Mark M.
so, get rid of the AsyncTask
Steve
We only want to use this custom AsyncTask that we use for all networking operation to add extra data/params when calling our endpoints
Mark M.
most likely, "all networking operation" shouldn't *be* an AsyncTask, but be something *used* by an AsyncTask
that way, that logic can also be used without an AsyncTask, such as by your IntentService
Steve
100% agreed
I just don't know if a refactor is in the question right now :P
Mark M.
personally, I have never called get() on an AsyncTask, as I break out in hives just thinking about it
I wouldn't expect a TimeoutException, though
Steve
Hahaha
get() essentially does not call doInBackground
I assumed it would just call doInBackground() on the calling thread
Mark M.
:: shrug ::
no
it should block the current thread waiting for the background thread to complete
remember, you called execute(), so that's kicking off the background thread
Steve
Ah got it
Mark M.
by the time you call get(), that background thread's already chugging along
Steve
That makes WAY more sense
Mark M.
now, I suspect that get() is... under-tested by Google, so it's possible that they introduced a bug somewhere along the line
Steve
And makes get() sound more useless :P
4:10 PM
Mark M.
well, it fits your need, as I understand it -- blocking until the work is completed
it just still involves a background thread
but, you could just call doInBackground() yourself directly
Steve
Yeah
Mark M.
replace the execute().get() with doInBackground() (followed by onPostExecute(), if you need that work too)
it wouldn't pass one of my code reviews, but, then again, I'm a harsh grader
:-)
Steve
It hurts me to think of doing that lol
We need to decouple the AsyncTasks from this logic at some point anyways :P
Mark M.
right
Steve
I'll take this back and see what the team thinks
Thanks for your help!
Mark M.
you're welcome!
Steve
Have a good one
Bye
Mark M.
see ya!
4:20 PM
Steve S.
has entered the room
Mark M.
hello, Steve S!
Steve S.
HI Mark!
Mark M.
the S-less Steve had some questions earlier but (AFAIK) is done at the moment
4:25 PM
Mark M.
so, it's your turn -- do you have a question?
Steve S.
sure. I'll paste in my question:
I'm trying to better understand how to use synchronization. Is closing a Bluetooth socket in a synchronized block something I should avoid?
Mark M.
um, I can't really answer that in the abstract
Steve S.
ok
Mark M.
what else is going on in the block?
Steve S.
here's the block:
View paste
 void cancel() {
            synchronized (LOCK) {
                mmIsCanceled = true;
                close(mmSocket);
            }
         }
This is used to cancel a read operation, which is blocking.
Mark M.
is close(mmSocket) doing much other than calling close() on the BluetoothSocket?
Steve S.
sorry - that's basically all that it's doing
Mark M.
is there anything else synchronizing on LOCK?
Steve S.
yes, there are about five other places where that's done
Mark M.
and some of this stuff might be called on different threads?
4:30 PM
Steve S.
yes - that's why i'm doing it
Steve
has left the room
Mark M.
then probably you should be synchronizing by one means or another
Steve S.
ok
Mark M.
it's tough to say without a deeper look at the code
Steve S.
sure
Mark M.
plus, my BluetoothSocket experience is limited (though it may increase here in the coming weeks)
if the only thing was mmIsCanceled, you could switch to an AtomicBoolean for that
Steve S.
i don't think that's an option in this case (i haven't shown you all the code...)
Mark M.
I guessed as mch
er, much
Steve S.
because some of the synchronized blocks are executed on the main thread, i assume i shouldn't include any potentially long-running operation in the synchronized blocks, like opening a connection
i was wondering if that would also be true of closing a connection
Mark M.
I have no idea how fast that is
my gut instinct would say it's cheap
usually, there isn't much handshaking on a disconnect in communications protocols, as there's no guarantee that a formal disconnect will ever happen
but, you'll need to do some performance analysis to see how quick it is in practice
4:35 PM
Mark M.
if it is sub-millisecond, don't worry about it
Steve S.
ok. i have done some timings, and i think the close operation takes about 3 ms
Mark M.
well, the per-frame budget is 16ms
on the one hand, 3 is quite a bit smaller than 16
on the other hand, there's other work to be done in the frame
if you don't mind the occasional frame drop on a close(), then don't worry about it
Steve S.
ok
Mark M.
for most frames with close(), the 3ms won't push you over budget, but occasionally it will
one of those probability things
Steve S.
regarding the 16ms budget: is that how long i have to do something on the main thread before returning control to the OS?
Mark M.
it's more that the UI updates, in a widget-based UI, are locked to a 60fps cycle
Steve S.
ok
Mark M.
so, every 16.666666666666666666ms, Android takes whatever has been updated in the buffer and blits it to the screen
(note: that number needs more 6's)
Steve S.
right!
Mark M.
if you take up too much time, nothing gets updated in the buffer, and so the UI does not change on that frame cycle
Steve S.
ok
Mark M.
now, it's entirely possible that nothing would change anyway, as the UI may not be drawing anything right then
but the drawing work is done by the framework on the main application thread
which is why we want all of our callbacks from the framework to our code to be cheap
because there might be a lot of them (e.g., getView() calls on a ListAdapter while scrolling)
4:40 PM
Steve S.
sure
Mark M.
in your case, close() isn't going to be called in a loop or other frequent cases, most likely
Steve S.
right
Mark M.
which means the odds are decent that the 3ms won't cause any particular problems
but, every now and then, something big might be happening in the UI when you decide to call close()
and if some animation (e.g., scrolling) doesn't update because of a dropped frame, you have a bit of jank
some people are very sensitive to jank -- I'm not one of them
an app can drop a fair number of frames before I start to notice
now, if close() were 30ms or 300ms, there's no debate: get that off the main application thread
and if it were 0.3ms, there's probably no debate: just don't worry about it
Steve S.
ok. so it sounds like i could get a better sense of whether it's ok to close the socket in a synchronized block by timing it, and as long as it doesn't add too much time, it would be ok
Mark M.
3ms is a tweener value
yup
Steve S.
cool. i'll do some more careful timings then
no more questions today - thank you for your help!
Mark M.
you're welcome!
Steve S.
have a great rest of the day!
Mark M.
now, I just need a third Steve to show up in the next 15 minutes, and I hit the trifecta!
4:45 PM
Steve S.
based on my experience, that shouldn't be all the unlikely
Steve S.
has left the room
4:55 PM
Mark M.
turned off guest access

Yesterday, November 29

 

Office Hours

People in this transcript

  • Mark Murphy
  • Steve
  • Steve S