[gui-dev] Re: [codepatch] A small optimization for SchemaReplyCollectionMapper

[Philippe Verdy]

There's a tradeoff here between using lazy initialization (requiring
synchronized instanciation) which boost aplication startup, and a very tiny
price related to synchronization of a method which is not so frequently
called: mapping schema replies occurs only when a user is receiving replies
to his own searches, and at most it would occur about 200 to 400 times per
search initiated by the user, and only provided that all results have
embedded meta-data.

In practice, a search will run for a couple of minutes and will return about
80-100 results with meta-data, so this accessor will be used on average
about 1-2 times per second. This is small enough to keep the
synchronizartion on this, with the benefit of shortening the application
launch time.

Remember that there are still tons of users running a PC with less than
128MB RAM, and not all running Windows XP on a fast processor > 1GHz: on
these system the application launch time, including the JVM startup and the
VM swap on disk, is quite long, and I have a 4-years old PC for which
LimeWire requires about 150 seconds to start and again 2 minutes to get
connected. This very notable for most notebook users (unless you use one of
the new very fast notebooks which now outperform many desktops).

----- Original Message -----
From: "Roger Kapsi" <***@kapsi.de>
To: <***@limewire.org>
Sent: Friday, July 23, 2004 12:30 AM
Subject: [codepatch] A small optimization for SchemaReplyCollectionMapper

While tracking a NPE in DAAP I saw that SRCMapper.instance() is
unnecessarily synchronized. This patch will surely save some
nanoseconds! :)
cu
Roger

----------------------------------------------------------------------------
----

_______________________________________________
codepatch mailing list
http://www.limewire.org/mailman/listinfo/codepatch

[Roger Kapsi]

Hi Philippe,

I'm maybe missing something but how does this has an impact on the
launch time or memory usage? In both cases the static class variable is
being initialized if SchemaReplyCollectionMapper is touched the first
time and never if no one touches it.

Roger


public class ClassA {
private static ClassA instance = new ClassA();

public static ClassA instance() {
System.out.println("ClassA.instance()");
return instance;
}

private ClassA() {
System.out.println("Constructor ClassA");
}
}

public class ClassB {
public static void main(String[] args) throws Exception {
System.out.println("ClassB.main()");
ClassA clazz = null;
System.out.println("Touch ClassA in 5 seconds...");
Thread.sleep(5000);
clazz = ClassA.instance();
}
}

Post by Philippe Verdy
There's a tradeoff here between using lazy initialization (requiring
synchronized instanciation) which boost aplication startup, and a very tiny
price related to synchronization of a method which is not so frequently
called: mapping schema replies occurs only when a user is receiving replies
to his own searches, and at most it would occur about 200 to 400 times per
search initiated by the user, and only provided that all results have
embedded meta-data.
In practice, a search will run for a couple of minutes and will return about
80-100 results with meta-data, so this accessor will be used on average
about 1-2 times per second. This is small enough to keep the
synchronizartion on this, with the benefit of shortening the
application
launch time.
Remember that there are still tons of users running a PC with less than
128MB RAM, and not all running Windows XP on a fast processor > 1GHz: on
these system the application launch time, including the JVM startup and the
VM swap on disk, is quite long, and I have a 4-years old PC for which
LimeWire requires about 150 seconds to start and again 2 minutes to get
connected. This very notable for most notebook users (unless you use one of
the new very fast notebooks which now outperform many desktops).
----- Original Message -----
Sent: Friday, July 23, 2004 12:30 AM
Subject: [codepatch] A small optimization for
SchemaReplyCollectionMapper

While tracking a NPE in DAAP I saw that SRCMapper.instance() is
unnecessarily synchronized. This patch will surely save some
nanoseconds! :)
cu
Roger

-----------------------------------------------------------------------
-----
----

_______________________________________________
codepatch mailing list
http://www.limewire.org/mailman/listinfo/codepatch

[Philippe Verdy]

----- Original Message -----
From: "Roger Kapsi" <***@kapsi.de>
To: "Philippe Verdy" <***@wanadoo.fr>
Cc: <***@gui.limewire.org>
Sent: Friday, July 23, 2004 8:01 PM
Subject: Re: [codepatch] A small optimization for
SchemaReplyCollectionMapper

Post by Roger Kapsi
Hi Philippe,
I'm maybe missing something but how does this has an impact on the
launch time or memory usage? In both cases the static class variable is
being initialized if SchemaReplyCollectionMapper is touched the first
time and never if no one touches it.

I know that, but I don't think there's any access to the instance accessor
before the user actually starts an active search, so this initialization is
delayed after application startup.
In the past, LimeWire was considered very slow to start because too many
classes needed to be loaded and instanciated to create singleton instances,
meaning that LimeWire could not start without loading, initializing and
instanciating immediately nearly all of its classes; on a system with less
than 128MB RAM, the launch time was really prohibitive, notably for PCs or
MAc with processors less than 1GHz.

This had an impact on what users feel about LimeWire: its immediate memory
footprint, intense VM swap at startup, and too many users disconnecting it
instead of letting it run in the background.

Of course many efforts have been invested in the core's code to reduce the
number of temporary objects allocated when LimeWire is running (notably many
temporary StringBuffer's or Strings, and tuning ArrayList's with a better
preallocation size to limit the number of reallocations).

Other efforts have been invested in creating accessors to reusable utility
objects like the XML parser, or the HTML engine, or the files hasher.

There are still efforts to put there, to maximize the reuse of temporary
work objects, notably in the handling of messages and their parsing (which
is today, along with draw buffers for the display, the most active source of
temporary objects that stress the VM garbage collector).

There are everywhere in the code many places where we can limit the use of
internal work objects, and this is visible when you create a VM snapshot and
consider their lifetime, or the large size of the active part of the VM.

On my Pentium Mobile Centrino 1,9HGz with 1MB RAM, LimeWire is a pleasure to
use, but this config is still far from the average config of candidate
LimeWire users. As long as LimeWire will be so CPU and RAM intensive, other
servents will compete with it (notably eMule, or even BearShare).

[Roger Kapsi]

Post by Philippe Verdy
I know that, but I don't think there's any access to the instance accessor
before the user actually starts an active search, so this
initialization is
delayed after application startup.

Huh? :-/ But the initialization is delayed in both cases after
application startup. As I said, the static variable gets only
initialized if someone calls SchemaReplyCollectionMapper.instance() the
first time and this is not different from the current implementation!?

Note: SRCM is initialized by MetaFileManager quite early at startup and
DaapMediator will make usage of it as well. So this class it's not
limited only to searches.

http://www.limewire.org/fisheye/viewrep/~raw,r=1.51.2.5/limecvs/daap/
com/limegroup/gnutella/gui/DaapMediator.java


Cheers
Roger

[Philippe Verdy]

Post by Roger Kapsi

Post by Philippe Verdy
I know that, but I don't think there's any access to the instance accessor
before the user actually starts an active search, so this
initialization is
delayed after application startup.

Huh? :-/ But the initialization is delayed in both cases after
application startup. As I said, the static variable gets only
initialized if someone calls SchemaReplyCollectionMapper.instance() the
first time and this is not different from the current implementation!?

OK. Anyway, this class is not so critical in my current usage pattern
snapshots, and it is not so complex that it will take much resource to
setup. So I agree that lazy initialization is not so fruitful here, and your
optimization simplifies the code...

Post by Roger Kapsi
Note: SRCM is initialized by MetaFileManager quite early at startup and
DaapMediator will make usage of it as well. So this class it's not
limited only to searches.

Thanks for pointing it, I did not search a lot within that part of the code,
and did not see that dependency.

As I said, most of the optimizations that remain to do is within the
Gnutella message handling code that use too many temporary objects, and that
limit the performance of UltraPeers, which still use too much memory when
they run in the background with many leaf connections:

I'm quite sure that we could reuse some message structures, at least within
the same connection thread (if we don't share these objects in a single pool
difficult to synchronize). This is will most useful for UP-to-UP
connections, that already handle a large volume of Gnutella messages with
too much memory footprint.

Also I don't like the way some messages are handled: they are parsed in
several levels, breaking a single message into several subobjects, notably
for the GGEP extension lists... Here again, we should be able to represent
all GGEP extensions within a single shared object for their backing store,
instead of breaking the message into many fragments with extra allocations
and copies:

- in each connection thread, the messages are handled one after the other,
and so each message could be handled in the same object than the previous
message already processed;
- its backing store should be directly accessible when parsing GGEP options;
- if an option needs to be decompressed, it should be done within a
decompression buffer reusable for the next message.
- when building new messages for responses, the connection thread should be
able to use its own pool of output buffers; we should be able to create
responses without using message constructors, but using that local pool and
sending it to pool accessors with the necessary options to fill the
messages...
- if needed, connections could be cleaned routinely to free up unused space
in their pool. This could be done by using the timeout parameter in the
connection listener: no message was received in the socket but the
connection thread can initiate this cleanup; this will help maintaining the
size of the internal pools low in each connection thread, so that an
ultrapeer with lots of connections will not explose its global memory with
lots of unused buffers in many threads.

[Sumeet Thadani]

Actually, the class will get loaded as long as it is referenced from
some class that has been loaded. The static initialization may slow it
down at startup, and also in the off chance that there is an error while
instantiating the instance, the class will fail to load.

I do not think the synchronization burden is too high though,
particularly since there is really no contention for the lock.

I am in favor of leaving it as is.

Thanks,

Sumeet

Post by Philippe Verdy

Post by Roger Kapsi

Post by Philippe Verdy
I know that, but I don't think there's any access to the instance accessor
before the user actually starts an active search, so this
initialization is
delayed after application startup.

Huh? :-/ But the initialization is delayed in both cases after
application startup. As I said, the static variable gets only
initialized if someone calls SchemaReplyCollectionMapper.instance() the
first time and this is not different from the current implementation!?

OK. Anyway, this class is not so critical in my current usage pattern
snapshots, and it is not so complex that it will take much resource to
setup. So I agree that lazy initialization is not so fruitful here, and your
optimization simplifies the code...

Post by Roger Kapsi
Note: SRCM is initialized by MetaFileManager quite early at startup and
DaapMediator will make usage of it as well. So this class it's not
limited only to searches.

Thanks for pointing it, I did not search a lot within that part of the code,
and did not see that dependency.
As I said, most of the optimizations that remain to do is within the
Gnutella message handling code that use too many temporary objects, and that
limit the performance of UltraPeers, which still use too much memory when
I'm quite sure that we could reuse some message structures, at least within
the same connection thread (if we don't share these objects in a single pool
difficult to synchronize). This is will most useful for UP-to-UP
connections, that already handle a large volume of Gnutella messages with
too much memory footprint.
Also I don't like the way some messages are handled: they are parsed in
several levels, breaking a single message into several subobjects, notably
for the GGEP extension lists... Here again, we should be able to represent
all GGEP extensions within a single shared object for their backing store,
instead of breaking the message into many fragments with extra allocations
- in each connection thread, the messages are handled one after the other,
and so each message could be handled in the same object than the previous
message already processed;
- its backing store should be directly accessible when parsing GGEP options;
- if an option needs to be decompressed, it should be done within a
decompression buffer reusable for the next message.
- when building new messages for responses, the connection thread should be
able to use its own pool of output buffers; we should be able to create
responses without using message constructors, but using that local pool and
sending it to pool accessors with the necessary options to fill the
messages...
- if needed, connections could be cleaned routinely to free up unused space
in their pool. This could be done by using the timeout parameter in the
connection listener: no message was received in the socket but the
connection thread can initiate this cleanup; this will help maintaining the
size of the internal pools low in each connection thread, so that an
ultrapeer with lots of connections will not explose its global memory with
lots of unused buffers in many threads.
_______________________________________________
gui-dev mailing list
http://www.limewire.org/mailman/listinfo/gui-dev
.