You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Nathan Beyer <nd...@apache.org> on 2010/01/13 04:21:22 UTC

interesting commentary via android bug report

http://code.google.com/p/android/issues/detail?id=5807

Re: interesting commentary via android bug report

Posted by Regis <xu...@gmail.com>.
On 2010-01-13 12:45, Regis wrote:
> On 2010-01-13 11:21, Nathan Beyer wrote:
>> http://code.google.com/p/android/issues/detail?id=5807
>>
>
> Harmony JIRA HARMONY-6417 raised for this issue. I'm more interesting on

Duplicate of HARMONY-6414, it's fixed already.

> the reasons of "a bad implementation of a bad idea".
>


-- 
Best Regards,
Regis.

Re: interesting commentary via android bug report

Posted by Regis <xu...@gmail.com>.
On 2010-01-13 11:21, Nathan Beyer wrote:
> http://code.google.com/p/android/issues/detail?id=5807
>

Harmony JIRA HARMONY-6417 raised for this issue. I'm more interesting on the 
reasons of "a bad implementation of a bad idea".

-- 
Best Regards,
Regis.

Re: interesting commentary via android bug report

Posted by Mark Hindess <ma...@googlemail.com>.
In message <4B...@gmail.com>, Regis writes:
>
> On 2010-01-13 13:49, Jesse Wilson wrote:
> > On Tue, Jan 12, 2010 at 7:47 PM, Charles Lee<li...@gmail.com>  wrote:
> >
> >> What's the reason of bad idea? limited memory?
> >>
> >
> > A cache that cannot be cleared is a disservice to users. It's an error in
> > our implementation (and the RI!) to assume that an application requesting
> > the canonical path two times wants to receive the same result. If the
> > application wants the same value, why doesn't the *application* cache the
> > result?!
> 
> canonical path is heavily used when checking file permission. So if
> a SecurityManager is installed, canonical path will be calculated by
> every file operation at least once, that the killer of performance,
> application cache can't help that.
> 
> >
> > If we want to make Harmony faster, there are better ways to do so. The
> > simpler optimization is to streamline the code that looks up the canonical
> > path *the first time*. Reducing the number of times we cross a JNI boundary
> > is one of many opportunities here.
> 
> Yes, as I know resolving symbol links part on Linux can be done in one
> JNI call.  But it's not conflict with the cache.

There are two obvious areas for improvement here:

1) moving the loop around the readlink function to native code to reduce
the JNI calls,

2) using the realpath syscall on platforms that support it to reduce the
number of syscalls

We should definitely do 1) (particularly as this is platform independent).

I think the realpath syscall is quite portable (using the second
parameter non-NULL which we'd want to use anyway) on Linux, Aix and
z/OS.  So unless there is a compelling reason not to use realpath we
should probably do 2) also.

While I agree with Jesse that it is a bad idea to have a cache with no
API o control it. the RI uses one so I think we'd be at a significant
disadvantage if we didn't use one as well.

It would be good to make the possible improvements and then confirm that
the disadvantage really is significant in practice.

Regards,
 Mark.



Re: interesting commentary via android bug report

Posted by Regis <xu...@gmail.com>.
On 2010-01-13 13:49, Jesse Wilson wrote:
> On Tue, Jan 12, 2010 at 7:47 PM, Charles Lee<li...@gmail.com>  wrote:
>
>> What's the reason of bad idea? limited memory?
>>
>
> A cache that cannot be cleared is a disservice to users. It's an error in
> our implementation (and the RI!) to assume that an application requesting
> the canonical path two times wants to receive the same result. If the
> application wants the same value, why doesn't the *application* cache the
> result?!

canonical path is heavily used when checking file permission. So if a 
SecurityManager is installed, canonical path will be calculated by every file 
operation at least once, that the killer of performance, application cache can't 
help that.

>
> If we want to make Harmony faster, there are better ways to do so. The
> simpler optimization is to streamline the code that looks up the canonical
> path *the first time*. Reducing the number of times we cross a JNI boundary
> is one of many opportunities here.

Yes, as I know resolving symbol links part on Linux can be done in one JNI call. 
But it's not conflict with the cache.

>
>
> What's the reason of bad implement? Anyone knows?
>>
>
> The canonical path cache code we shipped with Android 2.0 occasionally
> caused applications to crash with a NoSuchElementException.
>

It is fixed by HARMONY-6414.

-- 
Best Regards,
Regis.

Re: interesting commentary via android bug report

Posted by Charles Lee <li...@gmail.com>.
On Wed, Jan 13, 2010 at 1:49 PM, Jesse Wilson <je...@google.com> wrote:
> On Tue, Jan 12, 2010 at 7:47 PM, Charles Lee <li...@gmail.com> wrote:
>
>> What's the reason of bad idea? limited memory?
>>
>
> A cache that cannot be cleared is a disservice to users. It's an error in
> our implementation (and the RI!) to assume that an application requesting
> the canonical path two times wants to receive the same result. If the
> application wants the same value, why doesn't the *application* cache the
> result?!
>
> If we want to make Harmony faster, there are better ways to do so. The
> simpler optimization is to streamline the code that looks up the canonical
> path *the first time*. Reducing the number of times we cross a JNI boundary
> is one of many opportunities here.
>

IIRC, the reason we add the cache to the File is because RI seems much
faster than harmony. And reduce the number of times we cross a JNI
boundary improve the performance indeed, but still has a lot to
improve comparing with RI.
There is still a concern here: people who is using RI will not cache
the result in their application because getCanonicalPath() is fast in
RI.

>
> What's the reason of bad implement? Anyone knows?
>>
>
> The canonical path cache code we shipped with Android 2.0 occasionally
> caused applications to crash with a NoSuchElementException.
>

Yeah. It is a bug which has been fixed recently (HARMONY-6414). It
seems too late :-(


-- 
Yours sincerely,
Charles Lee

Re: interesting commentary via android bug report

Posted by Jesse Wilson <je...@google.com>.
On Tue, Jan 12, 2010 at 7:47 PM, Charles Lee <li...@gmail.com> wrote:

> What's the reason of bad idea? limited memory?
>

A cache that cannot be cleared is a disservice to users. It's an error in
our implementation (and the RI!) to assume that an application requesting
the canonical path two times wants to receive the same result. If the
application wants the same value, why doesn't the *application* cache the
result?!

If we want to make Harmony faster, there are better ways to do so. The
simpler optimization is to streamline the code that looks up the canonical
path *the first time*. Reducing the number of times we cross a JNI boundary
is one of many opportunities here.


What's the reason of bad implement? Anyone knows?
>

The canonical path cache code we shipped with Android 2.0 occasionally
caused applications to crash with a NoSuchElementException.

Re: interesting commentary via android bug report

Posted by Charles Lee <li...@gmail.com>.
Interesting.

What's the reason of bad idea? limited memory?
What's the reason of bad implement? Anyone knows?

On Wed, Jan 13, 2010 at 11:21 AM, Nathan Beyer <nd...@apache.org> wrote:
> http://code.google.com/p/android/issues/detail?id=5807
>



-- 
Yours sincerely,
Charles Lee