FWD » Core Development » Runtime Infrastructure

Many APIs between client and server have been added over the past few years, some of which pass new object types over the DAP. Not all of these have been made to implement the Externalizable interface. Allowing the default serialization to be used adds cost to both the serialization and deserialization of these objects, as well as increases the size of the data pushed over the network.

A comprehensive review is needed to identify all object types which are sent over the DAP, to see which need to be made Externalizable.

From Igor:

Please note however that in many cases which I've seen, the implementation of the Externalizable in FWD is pretty straightforward. I was working on the network protocol optimization for a <large_organization> in the past (it was very critical as the application was "almost" real-time). Using a more "hand-made" approach based on writeReplace/readResolve I'be managed to reduce the network traffic by an order of magnitude. Maybe it makes sense to do something like this, at least for the most "heavy" objects? Please note that it can be done for one object at a time.

This is interesting. I think it is worth exploring.

We also have the ideas from #3789.

Some useful references:

https://dzone.com/articles/java-serialization-magic-methods-and-use-cases
https://docs.oracle.com/javase/8/docs/technotes/guides/serialization/examples/symbol/index3.html
http://www.jguru.com/faq/view.jsp?EID=44039 (a more complete example of the writeReplace/readResolve combination)

I am assuming that as a quick first pass you are going to implement Externalizable in all of the classes passed over the network in our protocol. This should be quite straightforward and able to be completed quickly. I know this will show some benefit. Do you see any issue with being able to get some number of these done tomorrow?

1. SyncMessage is mostly used by FWD, and not plain Message. This has an additional Serializable state payload, which is either a ServerState or a ClientState. In SyncMessage, I think we should make this Externalizable state, and rely on the state.readObject(in) and state.writeObject(out).
2. ObjectInput.readObject and ObjectOutput.writeObject are still used for writing arrays, or java.lang. nullable fields. See their usage in ClientState and ServerState.
3. The Message.payload is usually an Object[] array, the arguments for a remote call. We need a smarter way to attach these.

Constantin Asofiei wrote:

Some other notes:

1. SyncMessage is mostly used by FWD, and not plain Message. This has an additional Serializable state payload, which is either a ServerState or a ClientState. In SyncMessage, I think we should make this Externalizable state, and rely on the state.readObject(in) and state.writeObject(out).
2. ObjectInput.readObject and ObjectOutput.writeObject are still used for writing arrays, or java.lang. nullable fields. See their usage in ClientState and ServerState.
3. The Message.payload is usually an Object[] array, the arguments for a remote call. We need a smarter way to attach these.

Reworking ServerState and ClientState (de)serialization

Re-worked ServerState and ClientState (de)serialization.
A quick test was performed with Hotel GUI and a large customer app.
Committed to 4011b revision 11581

Code Review Task Branch 4011b Revision 11581

The changes are good. Minor issues:

1. BrowseJasperDataSource, ColorSpec, ColorTable, ColorManager need history entries.

2. ColorTableEntry has a history entry but no changes.

Greg Shah wrote:

Code Review Task Branch 4011b Revision 11581

The changes are good. Minor issues:

1. BrowseJasperDataSource, ColorSpec, ColorTable, ColorManager need history entries.

2. ColorTableEntry has a history entry but no changes.

Fixed in revision 11582.

What is the next set of classes being worked?

Any classes related to browse or other large data transfer usage will be high impact.

Greg Shah wrote:

Any classes related to browse or other large data transfer usage will be high impact.

I understand. At this moment I'm trying to figure which classes are most "heavy". I understand that it should configs of complicated widgets Such as lists or browse. I see that the serialization of some of such configs is using writeObject.

Re-worked (de)serialization of the BaseConfig and its subclasses.
Tested with the Hotel GUI and a large customer application.
Committed tp 3821c revision 11470.
Working on the (de)serialization of remaining classes.

Re-worked (de)serialization of all classes transmitted over DAP from com.goldencode.p2j.ui package except TreeConfig.
Tested with Hotel GUI and a large customer application.
Committed to 3821 revision 11492.
Analyzing the remaining usage of readObject/writeObject.

Fixed the WidgetId instance (de)serialization when it is actually an instance of the WidgetDownId.

Committed to 3821c rev. 11495.

Fixed type parameters for Tree* classes

Committed to 3821c rev. 11501 and tree-derived widgets in <customer_application> rev. 284.

More (de)serialization changes committed to 3821c rev 11529.
This essentially finishes the re-work.
The commit also contains type parameters' fixes for Tree* classes.
Please review.
Thanks.

Do these changes have any dependencies in the large customer project (which subclasses the tree control) that must also be changed?

Greg Shah wrote:

Do these changes have any dependencies in the large customer project (which subclasses the tree control) that must also be changed?

The dependencies are minor and the corresponding changes are committed (see #3822)

Code Review Task Branch 3821c Revision 11529

The changes are quite good.

1. Please revert the extra debug output in Protocol.

2. The EnhancedBrowseConfig and related classes still need to be shifted to Externalizable. I haven't checked other cases, but it is important to find all of them.

3. TreeListConfig, TreeListNodeResource, TreeListWidget, TreeNodeCollection, TreeViewConfig, WidgetTree, TreeGuiImpl, TreeListGuiImpl, TreeViewGuiImpl are missing history entries (and maybe also copyright date changes).

4. AttachmentDescriptor, ClientParameters, TreeListNodeEntry need the copyright dates to be fixed.

Greg Shah wrote:

Code Review Task Branch 3821c Revision 11529

The changes are quite good.

1. Please revert the extra debug output in Protocol.

2. The EnhancedBrowseConfig and related classes still need to be shifted to Externalizable. I haven't checked other cases, but it is important to find all of them.

3. TreeListConfig, TreeListNodeResource, TreeListWidget, TreeNodeCollection, TreeViewConfig, WidgetTree, TreeGuiImpl, TreeListGuiImpl, TreeViewGuiImpl are missing history entries (and maybe also copyright date changes).

4. AttachmentDescriptor, ClientParameters, TreeListNodeEntry need the copyright dates to be fixed.

Fixed in revision 11530.
It appears that I've overlooked a lot of other classes implemented Serializable. Sorry. Working on them.

Igor, DatasetWrapper was changed that a string is read via readString instead of readObject, but there is a case where is still written via writeObject - see out.writeObject(resource.getStructureName());. Please double-check your changes that the read/write is done using the same approach.

Constantin Asofiei wrote:

Igor, DatasetWrapper was changed that a string is read via readString instead of readObject, but there is a case where is still written via writeObject - see out.writeObject(resource.getStructureName());. Please double-check your changes that the read/write is done using the same approach.

Thank you, Constantin. Of course I'm trying to double check my changes, but in this case I've overlooked a bug. Fixed in revision 11540.

Finished conversion from Serializable to Externalizable.
Committed to 3821c rev.11545

From Igor:

As of 3821c rev. 11545 I've finished re-work of Serilazable to Externalizable in FWD classes. This should reduce the network traffic and (in some cases) a (de)serialization overhead as the default Java Serializable implementation is notoriously inefficient.
Please note however that now adding new fields to the serializable classes require explicit changes in the readExternal/writeExternal methods. These changes may be needed if the type of the field is modified.
I've created a number of helper methods in the NativeTypeSerializer class to simplify this. A special efforts may be required if you add fields to a subclass of the Externalizable class or just create such subclass. The NativeTypeSerializer.Externalizer can help in this case.

Not sure I am posting to the right place, but I am having troubles building FWD with gradle all target in 3821c rev. 11545:

./gradlew clean all

Results in:

> Task :buildAdmin
Picked up _JAVA_OPTIONS: -Duser.timezone=GMT+3
Compiling module com.goldencode.p2j.admin.AdminApp
   Tracing compile failure path for type 'com.goldencode.p2j.security.Description'
      [ERROR] Errors in 'file:/home/vvt/p2j/branches/fwd/src/com/goldencode/p2j/security/Description.java'
         [ERROR] Line 272: No source code is available for type com.goldencode.util.NativeTypeSerializer; did you forget to inherit a required module?
         [ERROR] Line 290: No source code is available for type java.io.ObjectInput; did you forget to inherit a required module?
         [ERROR] Line 77: No source code is available for type java.io.Externalizable; did you forget to inherit a required module?
         [ERROR] Line 290: No source code is available for type java.lang.ClassNotFoundException; did you forget to inherit a required module?
         [ERROR] Line 266: No source code is available for type java.io.ObjectOutput; did you forget to inherit a required module?
      [ERROR] Errors in 'file:/home/vvt/p2j/branches/fwd/src/com/goldencode/p2j/security/BitSet.java'
         [ERROR] Line 73: No source code is available for type java.io.Externalizable; did you forget to inherit a required module?
... more errors

Vladimir Tsichevski wrote:

Not sure I am posting to the right place, but I am having troubles building FWD with gradle all target in 3821c rev. 11545:

It seems that I've changed too much and GWTCompiler doesn't understand this. Should I revert my changes and which classes should be reverted?
Thank you.

Hynek/Sergey: Please review #4026-30.

Greg Shah wrote:

Hynek/Sergey: Please review #4026-30.

I understand that the following classes should be left Serializable to be accepted by GwtCompiler.

  src/com/goldencode/p2j/admin/Acl.java
  src/com/goldencode/p2j/admin/AclDef.java
  src/com/goldencode/p2j/admin/AdminDef.java
  src/com/goldencode/p2j/admin/AdminProfile.java
  src/com/goldencode/p2j/admin/CertDef.java
  src/com/goldencode/p2j/admin/GroupDef.java
  src/com/goldencode/p2j/admin/ProcessDef.java
  src/com/goldencode/p2j/admin/RecordInfo.java
  src/com/goldencode/p2j/admin/RecordLockInfo.java
  src/com/goldencode/p2j/admin/SessionInfo.java
  src/com/goldencode/p2j/admin/StacktraceInfo.java
  src/com/goldencode/p2j/admin/TaggedName.java
  src/com/goldencode/p2j/admin/UserDef.java
  src/com/goldencode/p2j/admin/client/application/home/acl/model/AclRow.java
  src/com/goldencode/p2j/admin/client/editors/AdminRightsEditor.java
  src/com/goldencode/p2j/admin/client/editors/BitFlagsRightsEditor.java
  src/com/goldencode/p2j/admin/client/editors/DirectoryRightsEditor.java
  src/com/goldencode/p2j/admin/client/editors/NetRightsEditor.java
  src/com/goldencode/p2j/admin/client/editors/StringConditionRightsEditor.java
  src/com/goldencode/p2j/admin/client/editors/SystemRightsEditor.java
  src/com/goldencode/p2j/admin/shared/ReportPreview.java
  src/com/goldencode/p2j/admin/shared/ReportRequest.java
  src/com/goldencode/p2j/directory/DirectoryRights.java
  src/com/goldencode/p2j/net/NetRights.java
  src/com/goldencode/p2j/security/AdminRights.java
  src/com/goldencode/p2j/security/BitFlagsRights.java
  src/com/goldencode/p2j/security/BitSet.java
  src/com/goldencode/p2j/security/Description.java
  src/com/goldencode/p2j/security/Rights.java
  src/com/goldencode/p2j/security/RightsEditor.java
  src/com/goldencode/p2j/security/SessionToken.java
  src/com/goldencode/p2j/security/StringConditionRights.java
  src/com/goldencode/p2j/security/SystemRights.java

Igor Skornyakov wrote:

I understand that the following classes should be left Serializable to be accepted by GwtCompiler.
Is this correct?

Correct, AFAIK Externalizable is not supported by GWT.

Rolled back classes from #4026-32 in rev.11546.