[rfc][icedtea-web] Check online detection even if checked before

[Jie Kang]

----- Original Message -----
> ...snip...
> >>>>
> >>>>
> >>>>
> >>>> If -Xoffline is specified, I would like to keep current approach.
> >>>>
> >>>>
> >>>> For the online detection, I'm really concerned about isOnline result
> >>>> (and
> >>>> so
> >>>> behaviour of system)
> >>>>
> >>>>
> >>>> So brainstroming (valid only if no xOffline is specified):
> >>>>
> >>>> on begging assume environment is offline (no check was done)
> >>>> The check on "is online" will be done always from now
> >>>> If suddenly the resource is being avaiable online , switch to "online
> >>>> verified" and consider
> >>>> environment is online.
> >>>> No more checks are done from here.
> >>>>
> >>>> with Xoffline - no check is done ever.
> >>>>
> >>>> What do you think?
> >>>
> >>> Hello,
> >>>
> >>> If I understood correctly, I think the change to make it behave like
> >>> above
> >>> would be simple:
> >>>
> >>> In JNLPRuntime:
> >>>       public static void detectOnline(URL location) {
> >>>           if (onlineDetected != null) {
> >>>               return;
> >>>           }
> >>>
> >>> change:  if (onlineDetected != null) {
> >>
> >> [*] If so, then with related Boolean -> boolean change.
> >
> > Sorry can you explain this comment a little more?
> 
> 
> If the null will be never used, then it have to type of boolean, not Boolean.
> And I think it will
> not be. the default value now will be false.
> 
>   [***] I have now realized - how slow it will be if the environment really
>   will be offline? It will
> keep "pinging" all resources, and actually dying with timeouts. ... You will
> have to measures this
> on sme biger (all will start offline with current appracha and mostly
> sometimes dye later with
> runtimeexception)
> 
> There may be hiddden performance drop. (as opposite to current state, where
> is performance up when
> you plug out the cable)

Yes, you are right. This alternative change: if (onlineDetected == true)

will keep pinging resources and not have timeouts. I don't think it is a good solution anymore.


> >
> >>
> >>>       to:  if (onlineDetected == true) {
> >>>
> >>>
> >>> However, I wonder if we need to make it so complicated.
> >>>
> >>> I see two situations:
> >>>
> >>> A: Xoffline : no check is done ever and we never download : I think
> >>> ResourceTracker.initializeResource() will need some patching for this.
> >>> But
> >>> I already wrote one, just haven't sent it yet.
> >>>
> >>> B: 'Online' : for each resource, try online and if it fails, try cache,
> >>> otherwise error. This is already coded safely.
> >> I doubt.
> >>
> >>>
> >>>     My question here is: What is benefit of doing isOnline check? Where
> >>>     else
> >>>     is this used outside of initializing and downloading resources?
> >>
> >> Find usages is your friend.
> >>
> >> Especially the one in resourceTracekr is imho critical.
> >
> > Hahah, I think the opposite. The ones outside of ResourceTracker are great,
> > but the one in ResourceTracker is not as good.
> 
> And how will you proceed without them? I doubt it will work.
> >
> >>>
> >>>     When we use URLConnection object and try to initialize the resource,
> >>>     this already does a check for us. As well, the code already deals
> >>>     with
> >>>     the possible situations here.
> >>
> >> This is not working as smotthly as you expect. Please be careful with any
> >> fixes here, otherwise you
> >> will break offline capabilities ITW with effort achieved.
> >
> > Can you explain a little more? Possibly provide some examples and describe
> > what you mean by offline capabilities?
> 
> [**] That every part of ITW works even if you unplug the utp cable and turn
> of WiFi. Its crucial
> part of javaws implementation, which was long time (before offline patch)
> missing. As benefit, also
> appelts works offline. And it is something What I also wont to keep.

The original patch retains this behaviour, no?

> 
> >
> >
> >> 	
> >>>
> >>>     I do not think the isOnline check before trying to use URLConnection
> >>>     object has a performance benefit that justifies the extra complexity
> >>>     in
> >>>     code.
> >>
> >> No it does not. But make life much easier.
> >>>
> >>>
> >>>
> >>> Regards,
> >>
> >>
> >>
> >> Looking back to the orriginal patch which introduced offline capabilities:
> >>
> >> isOnlineDetected
> >> isOnline
> >>     and
> >> detectOnline
> >>
> >>
> >> and their's usages -
> >> 4 (+3 comfortable)
> >> 2
> >>     and
> >> 3
> >>
> >> it is not much, but the whole concept of those have to be revisited. If
> >> you
> >> grant that all the itw
> >> javaws and applets will work offline, than you can do whatever you need.
> >> But
> >> then I would recommand
> >> you to remove the offline capabilities patch, and implement the whole
> >> design
> >> on your own.
> >
> > I'd rather not remove the patch :\
> 
> But it may be best way, Remove it, and try to reimplemnt the offline solution
> with the bug you found
> fixed.
> >
> >>
> >>
> >> I'm quite afraid to change individual parts of this patch. And I do not
> >> wont
> >> some functions to
> >> return random values.
> >>
> >>
> >> Even the change of yours  [* above] may have side effects.  But It may be
> >> what is seek. Fix for your
> >> issue, and still keeping  all the behaviours I would like to have.
> >
> > I know for sure the [*] change will fix the issue I am having. I also
> > strongly think that it will keep the behaviours you'd like to have,
> 
> Ok. So well.. lets try it. (but be aware of [***])

After reading [***] I realize this new fix is incorrect.

> 
> I would like to know Jacob's opinion about [*] [**] and [***] :)
> 
>  > but this would need a bit more review. But, if you can specify exactly
>  > what behaviours you are
> looking for, then it will be easier to see if this fix works or not.
> >
> 
> explained  in [**]
> 
> > At the same time, I still feel a call to detectOnline() shouldn't return
> > just because it's been called before. I think the only cost of the
> > original patch is in performance and in my opinion it is extremely small.
> > It keeps pretty much everything you want, fixes my issue, and has one
> > drawback that I can see, which is performance. However, how big of a
> > performance drop is this? I don't think it's even close to noticeable.


After all this discussion, I believe the original patch is still the best choice. It does retain the features you specified, it doesn't have the issue of [***], and it fixes the bug.

The only issue I see with this patch is a clear performance drop. However, I must ask, is this really an issue? The time it takes for an online check is pretty miniscule, and on 'average' machines and networks, I would say unnoticeable to the user.

If you'd like, I can create + run some tests to see how much slower it is, but I feel like it will be in the millisecond range or smaller...

I have attached the patch again for reference.

Thoughts?


Regards,


> >
> >
> > Regards,
> >
> >>
>   J.
> 
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern
-------------- next part --------------
A non-text attachment was scrubbed...
Name: itw-online-detect-1.patch
Type: text/x-patch
Size: 552 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150206/ba2ffdb5/itw-online-detect-1.patch>