[rfc][icedtea-web] Check online detection even if checked before
Jiri Vanek
jvanek at redhat.com
Mon Feb 9 10:51:46 UTC 2015
On 02/06/2015 03:22 PM, Jie Kang wrote:
>
>
> ----- 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.
If you can swear that offline capabilities are not broken by this (which I doubt a bit!) then I'm
inclined to try your approach.
>
> 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.
I agree with you.
>
> 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.
>
The issue I still see is the starnge result of isOnline()
With your changesst, the original behavior is impossible to achieve. So even If I'm now convinced
that what you do is right, it seems to be not completed.
The methods of isOnline and isOnlineDetected are depnding on state of onlineDetected and logic of
ITW is depnding on its result.
My idea is:
- keep your changeset - always repeat detectOnline, unless xoffline is specified
- keep onlineDetected as false - once moved to true, make it impossible to set it back to false
- this may have side effects when network is somehow closed iin runtime, but afiak this is not
supported even now (nor ever?)
Thoughts? Actually ideas how it will behave?
Update.... After playing with it I found that what I suggested is breaking file:// handling
Something liek attached had worked for me. 9note - try tosolve it without looking to my "patch" I
dont like my approach.
btw - I found a bug in my -html switch. It works offline only when Xoffline is specified... weird...
J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: deterministicOnline.patch
Type: text/x-patch
Size: 1254 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150209/2aa3bbe8/deterministicOnline.patch>
More information about the distro-pkg-dev
mailing list