Issue 15966: XML-Based QoS Policy Settings (DDS-PSM-Cxx/DDS-PSM-Java)
Issue 15968: formal description of how topic types are mapped to Java classes needed
Issue 16050: duplicate put definition resulting in a name clash
Issue 16056: Data access from DataReader using java.util.List
Issue 16104: Missing -behavioural- descriptions of the interface
Issue 16316: Improve usability of “bucket” accessors
Issue 16317: Update specification to reflect DDS-XTypes FTF1 issue resolutions
Issue 16318: Entity.setListener is missing listener mask
Issue 16319: Unclear blocking behavior for WaitSet.waitForConditions overloads that don’t specify timeout
Issue 16320: Incorrect topic type specification in DomainParticipant.createMultiTopic
Issue 16321: Too many read/take overloads
Issue 16322: DynamicDataFactory.createData missing a parameter
Issue 16323: Logically ordered types should implement java.lang.Comparable
Issue 16324: Improve polymorphic sample creation
Issue 16325: Remove unnecessary DataWriter.write overloads
Issue 16326: copyFromTopicQos signatures are not correct
Issue 16327: Parent accessors should be uniform across Entities and Conditions
Issue 16328: DataReader.createReadCondition() is useless
Issue 16369: QosPolicy.Id enumeration is redundant
Issue 16529: Modifiable Types should be removed and replaced by values (e.g. immutable types).
Issue 16530: Superfluous "QoSPolicy" Suffix on Policy Types
Issue 16531: Getting rid of the Bootstrap Object
Issue 16532: RxO QoS Policies should be Comparable (idem for QoS)
Issue 16533: Qos Policies ID class vs. numeric ID
Issue 16534: Constant Values SHALL be defined by the standard
Issue 16535: Large Number of Spurious Import
Issue 16536: QoS DSL Needed
Issue 16537: Get rid of the EntityQos Class
Issue 16538: Entity class allows for breaking invariants
Issue 16539: DomainEntity should be removed
Issue 16540: DataReader API
Issue 16541: A Status is not an Event. An Event is not a Status, it notifies a status change
Issue 16542: Avoid unncessary side-effects on the DataWriter API
Issue 16543: Statuses API shoud be improved and made typesafe
Issue 16587: API Should Avoid Side-Effects, e.g. Remove Bucket Accessors
Issue 16588: The Sample class should provide a method to check wether the data is valid
Issue 16589: Misnamed Listener Helper
Issue 17065: Class for Query Expression
Issue 17204: Obsolete EntityQos interface name
Issue 17302: Implement Java5 Closeable interface
Issue 17303: Update specification for final DDS-XTypes
Issue 17304: Improve compile-time type safety of EntityQos
Issue 17415: Implement java.io.Closeable in Sample.Iterator
Issue 18285: Redundant "QosPolicy" suffix on Policy Types
Issue 15966: XML-Based QoS Policy Settings (DDS-PSM-Cxx/DDS-PSM-Java) (dds-psm-java-ftf)
Click here for this issue's archive.
Source: PrismTech (Dr. Angelo Corsaro, PhD., angelo.corsaro(at)prismtech.com)
Nature: Uncategorized Issue
Severity: Minor
Summary:
ISSUE
The newly introduced XML Based Policy configuration adds new methods in the core DDS entities that allow to fetch QoS from XML filers. This solution is not ideal since if generalized, e.g. QoS configuration from an URI, JSON stream, etc., would lead to an explosion of the core DDS API.
The suggestion is to remove the added methods from the core API and use instead a Builder pattern (of some form).
A sketch of the suggested change is provided below:
PolicyBuilder builder = PolicyBuilder::load("XMLBuilder");
TopicQos tqos = builder.topic_qos(file_name, profile_name);
==============================================================================
Notice that the suggested approach allows to easily extend the supported format for QoS representation w/o any impact on the core DDS API and overall facilitate the support for multiple approaches.
The DDS-PSM-Java currently provides examples of the the new mapping from the DDS type system to the Java programming language but does not provide a formal description of how topic types are mapped to Java classes. This underspecification should be filled to align the DDS-PSM-Java with the DDS-PSM-Cxx and to ensure that different/old mappings are used by DDS implementations.
[Rick] Note that DDS-PSM-Cxx does not require implementations to use the new Plain Language Binding it defines; that binding is an optional conformance point. I believe that’s the right model to follow in DDS-PSM-Java as well. The FTF membership discussed this issue at the Orlando meeting and agreed to the following principles: 1. This issue overlaps the existing scope of the specification (i.e. the existing Java Type Representation) sufficiently that the issue should be accepted and resolved. 2. The Plain Language Binding for Java would map type members to Java Bean-style property accessors and mutators. This pattern is familiar to Java programmers, understandable to type designers, and consistent with the approach taken in the DDS-PSM-Cxx spec and the forthcoming IDL-to-C++11 specifications. 3. Ideally, the application of the Java Type Representation and the Plain Language Binding should be idempotent. In other words, if a type designer (a) starts with a Java class, (b) infers from it a type in the DDS type system (according to the Java Type Representation), and then (c) maps that abstract type back to a Java class (according to the Plain Language Binding), the result should be the same as the type he started with in (a) (or very close to it). The implication of the above principles is that unfortunately the Java Type Representation will need to change significantly. Since this realization comes late in the process, and we intend to charter a second FTF anyway (because of the schedule of DDS-XTypes, on which this specification depends), it is prudent to defer this issue until it can be resolved carefully and thoroughly. Resolution: Defer this issue. Disposition: Deferred
The ModifiableEntityQos contains put() definition that, after type erasure, cannot be distinguished from the inherited put definition in EntityQos (or the one inherited from Map) resulting in duplicate definitions of put: QosPolicy<?,?> put(QosPolicy.Id, QosPolicy<?,?>) Possible ways to resolve this: ·Drop the “extends Map” in EntityQos and put a dedicated get() in EntityQos and a dedicated put()/set() in ModifiableEntityQos and leave it up to the implementation on how to manage these values. This is the preferred solution as it prevents the user of the API to accidently use the Map inherited modification methods like put/remove/clear on a non-modifiable EntityQos. ·Modify the signature of put() in ModifiableEntityQos to match the inherited definitions in EntityQos and Map: public QosPolicy<?,?> put(QosPolicy.Id key, QosPolicy<?,?> value);
André] Possible ways to resolve this: 1. Drop the “extends Map” in EntityQos and put a dedicated get() in EntityQos and a dedicated put()/set() in ModifiableEntityQos and leave it up to the implementation on how to manage these values. This is the preferred solution as it prevents the user of the API to accidently use the Map inherited modification methods like put/remove/clear on a non-modifiable EntityQos. 2. Modify the signature of put() in ModifiableEntityQos to match the inherited definitions in EntityQos and Map: public QosPolicy<?,?> put(QosPolicy.Id key, QosPolicy<?,?> value); [Rick] I think the Map extension provides a useful way to navigate QoS objects in a generic way. Therefore, I prefer the second approach. See also issue #16369, which will potentially impact the same method signature. See also issue #16537, which proposes eliminating the EntityQos interface altogether. See the following revisions, which contain provisional fixes for this issue: • Revision #116: http://code.google.com/p/datadistrib4j/source/detail?r=116, which updates the method signature as described in (2) above and also addresses issue #16369. • Revision #136: http://code.google.com/p/datadistrib4j/source/detail?r=136, which further refines the method signature to address a compilation error introduced by Java 7 compliance.
Currently the DataReader provides read() and take() methods that return a special type of java.util.ListIterator: Sample.Iterator. The Iterator is not the most convenient way to access data retrieved from the DataReader (e.g. an Iterator can only be traversed once).
Propose to modify all read()/take() operations currently returning an Iterator to let them return a java.util.List. The List is more developer friendly, as it can be traversed multiple times and a List is also an Iterable with the added benefit that it can be used in Java’s “foreach” statement:
List<Sample<TYPE>> data = dataReader.read();
for (Sample<Type> sample : data) {
// ...
}Some parts of the interface (javadoc) are poorly documented especially with respect to behaviour. This Java documentation will be the key documentation for the -new- DDS application programmers. It may be trivial or implicit for the ones writing the standard but it will not be for the application programmers which are not familiar with the existing DDS standard will use it For example have a look at the method createDataWriter(Topic<TYPE> topic) on the Publisher interface. What will happen if the middleware cannot create the datawriter. Will an unchecked exception be thrown or is a null value returned or even worse the datawriter is simply returned and will fail when the first write action is performed? I now that the existing OpenSplice DDS implementation will return null when the middleware is not able to create the datawriter but it would be nice that applications are not only portable from interface compliance aspect but also from behavioural aspect! (and that application programmers are aware of it)
[Rick] The behavioral specifications already exist—in the appropriate specification documents: DDS, DDS-XTypes, and DDS-PSM-Java. So I think this issue is not about portability, but really about ease of use: it is more convenient to programmers if more of the relevant specifications are available copied into the JavaDoc.
The third bullet at the end of section 7.1.5, “Method Signature Conventions”, reads: · Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind. (This pattern of passing a container to an object for that object to “fill in” has sometimes been referred to as a “bucket” pattern.) In cases where object-allocation performance is not a significant concern, the usability of this pattern can be improved with a trivial addition: allow the caller to pass in a null “bucket”, and require the implementation to allocate and return a new object with the appropriate contents. Proposed Resolution: Add a sentence to the bullet that indicates that a null argument is permitted. Proposed Revised Text: Replace the third bullet in section 7.1.5, “Method Signature Conventions” with the following: · Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. The caller may alternatively pass a null argument into such accessor methods, in which case the implementation shall allocate a new object, set its contents appropriately, and return it. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind.
Discussion: [Rick] To resolve this issue by itself, we could add a sentence to the bullet that indicates that a null argument is permitted. Specifically, replace the third bullet in section 7.1.5, “Method Signature Conventions” with the following: • Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. The caller may alternatively pass a null argument into such accessor methods, in which case the implementation shall allocate a new object, set its contents appropriately, and return it. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind. Resolution: Issue #16587 also deals with bucket accessors and has a broader scope. This issue should be merged with that one. Disposition: Merged with issue #16587
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The (first) DDS-XTypes FTF has completed. Some of the issue resolutions result in API changes that impact this specification. Those changes should be reflected here to keep this specification aligned with the PIM. These issues potentially include: · #15689, Identifiers TypeId and Module collide with IDL keywords · #15691, Unclear member names when programming language doesn’t support typedef · #15693, Extensibility kinds of new QoS policies are not specified in a consistent way · #15696, Incorrect FooDataWriter overloads for built-in types · #15706, Reduce size of DynamicData API Note that this issue applies to DDS-PSM-Cxx too.
Source:
RTI (Rick Warren, rick.warren@rti.com)
Summary:
The method signature for Entity.setListener does not include the listener “mask” (actually, a collection of status classes in this PSM) parameter from the PIM.
Discussion:
The current signature is useful as a convenience for the common case where the application wants all callbacks. But it lacks the expressiveness of the PIM, so an additional overload should be provided.
Proposed Resolution:
Add the following method to the Entity interface:
public void setListener(
LISTENER listener,
Collection<Class<? extends Status<?, ?>>> statuses);
Include the appropriate JavaDoc copied from the DDS specification
The current signature is useful as a convenience for the common case where the application wants all callbacks. But it lacks the expressiveness of the PIM, so an additional overload should be provided.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The method WaitSet.waitForConditions is provided with several overloads, including some that do not take an explicit timeout. These are intended to wait indefinitely. However, they still throw TimeoutException. How can they time out if they wait forever? Discussion: Object.wait allows indefinite waiting, so it makes sense for this specification to allow it as well. However, these overloads should not ever throw TimeoutException. Proposed Resolution: Remove the clause “throws TimeoutException” from these method declarations.
Object.wait allows indefinite waiting, so it makes sense for this specification to allow it as well. However, these overloads should not ever throw TimeoutException.
The method DomainParticipant.createMultiTopic specifies the type of the resulting object using a registered type name in string form. However, this is inconsistent with the way type registration is handled elsewhere in this PSM: callers provide a Class or TypeSupport object, and the implementation registers the type implicitly as necessary. Discussion: We should follow the model of createTopic: provide two overloads, one taking a Class and the other a TypeSupport. Proposed Resolution: Replace the existing createMultiTopic method declaration with two new overloads. In place of the typeName string, the first new overload shall take a Class parameter. The second shall take a TypeSupport parameter
We should follow the model of createTopic: provide two overloads, one taking a Class and the other a TypeSupport
The DataReader interface defines a very large number of read and take variants. While each one has a clear meaning, the sheer number of them makes the API harder to understand. Discussion: One possibility would be to follow the example of the C++ PSM, and combine things like condition, handle, etc. into a “filter” parameter. Note that this issue should be resolved at the same time as #16056.
[Rick] One possibility would be to follow the example of the C++ PSM, and combine things like condition, handle, etc. into a “filter” parameter. [Angelo] The only overload for the createReadCondition should be accepting a parameter that is the same as the DataReader.read e.g. a Query. This way the API will be more consistent. See the following revisions, which constitute a provisional resolution to this issue: • Revision #131: http://code.google.com/p/datadistrib4j/source/detail?r=131, which collapses view states, instance states, sample states, and other parameters into helper types Subscriber.ReaderState and DataReader.Query. • Revision #135: http://code.google.com/p/datadistrib4j/source/detail?r=135, which eliminates mandatory memory allocations in some overloads. • Revision #152: http://code.google.com/p/datadistrib4j/source/detail?r=152, which makes the signatures of createReadCondition and createQueryCondition consistent with the new signatures of read and take.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: According to DDS-XTypes, the operation DynamicDataFactory.create_data (createData in this PSM) takes a parameter that indicates the DynamicType of the new data object to create. That parameter is missing, leaving implementations with no way to determine—and applications with no way to specify—the type of the created object. Proposed Resolution: Add the missing argument. Proposed Revised Text: Apply the following difference to the method signature: - public abstract DynamicData createData(); + public abstract DynamicData createData(DynamicType type);
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: Several of the types defined in this PSM have a natural order (such as Time). In order to better integrate with the Java platform, these types should implement the standard java.lang.Comparable interface. Proposed Resolution: Implement Comparable in the following types: · Bit—ordered based on index within a bit set · Duration—ordered from shorter durations to longer ones · Time—ordered from earlier points in time to later ones
See the following revisions, which constitute a provisional resolution to this issue: • Revision #122: http://code.google.com/p/datadistrib4j/source/detail?r=122. This update encompasses Bit, Duration, and Time. • Revision #134: http://code.google.com/p/datadistrib4j/source/detail?r=134. This update encompasses InstanceHandle.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The specification does not provide a simple, portable way to create a new data sample to use with the middleware. Instead, there are several partial solutions: · Instantiate a concrete sample type directly: “new Foo()”. This approach doesn’t work in generic methods—i.e. when the concrete type is not statically known. It also doesn’t work with DynamicData. · Instantiate DynamicData from DynamicDataFactory. But samples of statically known, user-defined types don’t have a “data factory”. · Use DataReader.createData(). But there is not equivalent on the publishing side. There should be a single way that works uniformly and generically. Proposed Resolution: The proposed resolution has several parts: 1. Introduce a new factory instance method to the TypeSupport class: TypeSupport.newData(). The name of this method is parallel to that of other value type “constructor-like” factories. 2. Support navigation from the TopicDescription to the TypeSupport. Add a new method TopicDescription.getTypeSupport(). 3. Simplify the number of ways to get from the data type’s TypeSupport to its Class. Add a method TypeSupport.getType(). Remove the existing methods TopicDescription.getType(), DataWriter.getType(), and DataReader.getType(): they are redundant. 4. Remove the existing method DataReader.createData() and the existing class DynamicDataFactory. They are not needed. There will therefore be a single polymorphic and generic way to instantiate a sample of any type: by using its TypeSupport. (You can get the TypeSupport from any related TopicDescription, or transitively any DataReader or DataWriter.) Likewise, there will be a single polymorphic and generic way to get the Class object for any data type: from its TypeSupport. As described in the previous paragraph, you can get to the TypeSupport from a variety of places.
Title: Remove unnecessary DataWriter.write overloads Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The specification currently provides overloads for DataWriter.write that take the following combinations of parameters 1. The sample to write, without an instance handle. (If the type is not keyed, no instance handle is necessary. If it is keyed, the instance handle is implicitly nil and will be inferred by the implementation.) 2. The sample to write, without an instance handle but with a time stamp. 3. The sample to write, with an instance handle. 4. The sample to write, with both an instance handle and a time stamp. The overloads would be easier to understand if they formed a progression from fewer parameters to more. We can do this by removing (2). Proposed Resolution: Remove the following methods: - public void write( - TYPE instanceData, - Time sourceTimestamp) throws TimeoutException; - public void write( - TYPE instanceData, - long sourceTimestamp, - TimeUnit unit) throws TimeoutException; Also, update the documentation of the remaining overloads to clarify that if the topic is not keyed, they can be called with a nil or null InstanceHandle.
[Rick] Proposal: Remove the following methods: - public void write( - TYPE instanceData, - Time sourceTimestamp) throws TimeoutException; - public void write( - TYPE instanceData, - long sourceTimestamp, - TimeUnit unit) throws TimeoutException; Also, update the documentation of the remaining overloads to clarify that if the topic is not keyed, they can be called with a nil InstanceHandle. [Virginie] The instance handle on one hand and the timestamp on the other hand are two orthogonal parameters; I find it rather confusing to introduce here a “fake ordering” between those two things that are totally unrelated. If we were to remove unnecessary overloads, why not selecting just one way to express timestamps instead of the two currently existing? [Angelo] Not sure this is the right approach of reducing overloads. Perhaps the approach is to have only one way of passing a time object. Yet, it is better to avoid API that allow for parameters to be NIL or null. It is not clear that the current set of overloads is a problem. This issue is closed without any changes. Disposition: Closed, no change
Proposed Resolution: Change the signatures as follows: In Publisher.java: - public void copyFromTopicQos(DataWriterQos dst, TopicQos src); + public ModifiableDataWriterQos copyFromTopicQos( + ModifiableDataWriterQos dst, TopicQos src); In Subscriber.java: - public void copyFromTopicQos(DataReaderQos dst, TopicQos src); + public ModifiableDataReaderQos copyFromTopicQos( + ModifiableDataReaderQos dst, TopicQos src);
See revision #126: http://code.google.com/p/datadistrib4j/source/detail?r=126, which constitutes a provisional resolution to this issue. Resolution: Change the signatures as follows: In Publisher.java: - public void copyFromTopicQos(DataWriterQos dst, TopicQos src); + public ModifiableDataWriterQos copyFromTopicQos( + ModifiableDataWriterQos dst, TopicQos src); In Subscriber.java: - public void copyFromTopicQos(DataReaderQos dst, TopicQos src); + public ModifiableDataReaderQos copyFromTopicQos( + ModifiableDataReaderQos dst, TopicQos src);
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: All DomainEntity interfaces, and some Condition interfaces, can provide a reference to the parent object. In the case of Entities, this accessor has been captured in the form of a generic base interface method: PARENT DomainEntity.getParent(); However, StatusCondition and ReadCondition are not parallel. They provide the following methods: ENTITY StatusCondition.getEntity(); DataReader<TYPE> ReadCondition.getDataReader(); It would be more consistent if we renamed both of the above methods to getParent. Proposed Resolution: Rename StatusCondition.getEntity to getParent. Rename ReadCondition.getDataReader to getParent.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The DataReader interface provides two overloads for the createDataReader method: one that takes no arguments and another that takes the appropriate sample states, view states, and instance states. The existence of the first overload supposes that it will be common to create a ReadCondition with any sample state, any view state, and any instance state. But in fact, such a ReadCondition is not very useful at all: there’s no point in passing it to read/take, because it will not filter the available samples in any way. And although you could use it with a WaitSet, it doesn’t do anything that you couldn’t do with a StatusCondition on the DATA_AVAILABLE status. Proposed Resolution: Remove the no-argument overload of DataReader.createReadCondition. Leave the three-argument overload unchanged.
In the DDS PIM, each QoS policy has a name and an ID that uniquely identify it. In this PSM, these two things are encapsulated in the enumeration QosPolicy.Id. But the Java platform already provides equivalent information: the Class object. The ability to quickly compare Class object pointers is equivalent to comparing ID integer values, and each Class already has a name string. Proposed Resolution: Remove the enumeration QosPolicy.Id. Replace its uses with Class<? extends QosPolicy>.
See the following revisions, which contain a provisional resolution to this issue: • Revision #116: http://code.google.com/p/datadistrib4j/source/detail?r=116 • Revision #121: http://code.google.com/p/datadistrib4j/source/detail?r=121
The dds-psm-java introduces modifiable versions for conceptually immutable classes as a way to safe a few object allocations. However this is done for QoS which are not changed so often and that are overall very "thin" object. [Suggested Resolution] The proposed resolution is to get rid of this modifiable types and to ensure that value types are used everywhere. Althought this solution might lead to think that immutable types induce the creation of more objects this is not necessarily the case if the API si designed carefully as done for policies and Qos on simd-java (see git@github.com:kydos/simd-java.git). As an example, with the API included in the current DDS-PSM-Java modifying a policy would require the following steps: // Get unmodifiable QoS for inspection: DataWriterQos udwq = dw.getQos(); // Get the Modifiable QoS ModifiableDataWriterQos mdwq = udwq.modify(); // Modify the Qos mdwq.setReliability(...); With immutable Policies and QoS the same code could be rewritten as follows: DataWriterQos dwq = dw.getQos().with(Reliability.Reliable()); But you could also do: DataWriterQos dwq = dw.getQos().with( Reliability.Reliable(), Durability.Transient()); Notice that both code fragment both lead the lead the creation of a single new object. Yet the propsed approach not only gets rid of the complexity of the mutable objects, but it also get rids of the danger introduced by having mutable objects into multi-threaded applications. In summary, the proposed change (1) simplifies the API, (2) makes it safer, and (3) does not introduce runtime overhead (it actually allows for an higher degree of object sharing and thus better space efficiency). NOTE: Cloneable interface No need to implement the interface once the mutable pkg is removed
[Angelo] The proposed resolution is to get rid of these modifiable types and to ensure that value types are used everywhere. Although this solution might lead to think that immutable types induce the creation of more objects this is not necessarily the case if the API is designed carefully as done for policies and QoS on simd-java (see git@github.com:kydos/simd-java.git). As an example, with the API included in the current DDS-PSM-Java modifying a policy would require the following steps: // Get unmodifiable QoS for inspection: DataWriterQos udwq = dw.getQos(); // Get the Modifiable QoS ModifiableDataWriterQos mdwq = udwq.modify(); // Modify the Qos mdwq.setReliability(...); With immutable Policies and QoS the same code could be rewritten as follows: DataWriterQos dwq = dw.getQos().with(Reliability.Reliable()); But you could also do: DataWriterQos dwq = dw.getQos().with( Reliability.Reliable(), Durability.Transient()); Notice that both code fragments lead to the lead the creation of a single new object. Yet the proposed approach not only gets rid of the complexity of the mutable objects, but it also get rids of the danger introduced by having mutable objects into multi-threaded applications. In summary, the proposed change (1) simplifies the API, (2) makes it safer, and (3) does not introduce runtime overhead (it actually allows for an higher degree of object sharing and thus better space efficiency). NOTE: Cloneable interface: No need to implement the interface once the mutable package is removed [Rick] Situational analysis: • The biggest occurrence of the modifiable/unmodifiable pattern is in the QoS policies and Entity QoS. • ModifiableDuration can easily go away. Duration is only returned from QoS policy property accessors; QoS policies are not performance-sensitive. And in every case where durations are passed as arguments, there are already overloads to use an integer and a TimeUnit. • ModifiableTime is used in two places: DomainParticipant.getCurrentTime and Sample.getSourceTimestamp. Both are performance-sensitive, although the latter could potentially be replaced by simply Time. Time is accepted as an argument in a number of DataWriter methods, though these can be easily eliminated: each already has an overload that accepts an integer and a TimeUnit. • ModifiableInstanceHandle is used in statuses and in lookupInstance, where it needs to support being copied over. However, other values—like the nil handle constant, Entity instance handles, and the result of registerInstance—should not be changed. All of these APIs can be performance-sensitive. • AnnotationDescriptor and MemberDescriptor from the Dynamic Type API are provided in modifiable and unmodifiable versions. This API is not performance-sensitive, so accessors could simply return new copies of modifiable types. Orlando meeting discussion: • Consider replacing this pattern with a more explicit Builder pattern (see issue #15966) and/or a DSL (see issue #16536) in the case of Entity QoS and QoS policies. • Eliminate ModifiableDuration and leave Duration as an immutable type. Eliminate method overloads that accept Duration as an argument, leaving in place those that accept an integer and a TimeUnit. • Implement a lighter-weight version of this pattern specifically for Time and InstanceHandle rather than retaining it for all value types. To avoid race conditions, these classes should NOT be related by inheritance. • Remove AnnotationDescription, renaming ModifiableAnnotationDescriptor to AnnotationDescriptor. Remove MemberDescription, renaming ModifiableMemberDescriptor to MemberDescription. • Remove all “modifiable” packages. Resolution: Defer this issue. This issue was filed after the comment deadline, and its resolution will have a broad impact and has dependencies on the resolutions of other issues. It will be better to address it later, when there is sufficient time to make and review the changes thoroughly. Disposition: Deferred
The dds-psm-java uses a superfluous Policy suffix to name the DDS policies which themselves are already included in a "policy" namespace. [Resolution] This suffix should be removed.
The boostrap class is a pain for users which is in place only to allow users to run 2 different DDS implementations on the same application. The introduction of the Bootstrap object makes it impossible to use natural constructors for creating DDS types, even for types such as Time and Duration. As one of the main goal of the new DDS PSM was to simplify the user experience and make the API as simple and natural as possible, it seems that the introduction of the Bootstrap object goes exactly on the opposite direction -- all of this to be able to cover the case in which a user wants 2 different DDS implementation on the same application. Considering the wire-protocol interoperability this use case seems marginal and perhaps does not even count for 1% of DDS uses. [Resolution] The bootstrap should be removed and the resulting API should be simplified.
The concern for simplicity and usability expressed above is valid and important. The response to this concern will be constrained by requirements, including those below. In particular, the statement above that the Bootstrap class exists only to enable single applications to use two DDS implementations is incorrect. Requirements: The Bootstrap class is designed to avoid the brittle mixing of concrete implementation with abstract specification, which would occur if either the specification mandated implementation or if vendors re-implemented different classes with the “same” names. In addition, it is designed to enable the following deployment scenarios: 1. Standalone deployment of a single application using one or more DDS implementations. (The expected use case for multiple implementations is a DDS-to-DDS bridge.) 2. Deployment within an OSGi container, which may host multiple independent applications. a. More than one of application may use DDS internally, unknown to those applications. b. A given application should allow the injection of a concrete DDS dependency—that is, it should be able to declare, “I depend on DDS” and allow the platform’s administrator to choose the implementation. (This implies that the DDS API itself and the concrete implementation must be deployable as separate bundles, with multiple “application” bundles all depending on a common “specification” bundle.) 3. Deployment within a Java EE container, including one that—like JBOSS Application Server—uses a unified class loader design. (Such a deployment is like [1] above, although it expands the use case for multiple DDS implementations beyond bridges, as in [2a].) (As a point of comparison, the above needs are met by JMS.) Design Implications: The above requirements have two implications: ? They forbid the keeping of any static state or static dependency on any concrete implementation. Specifically, state pertaining to multiple DDS implementations must be able to coexist within a single class loader in order to support OSGI dependency injection, deployment to JBOSS AS, or DDS-to-DDS bridges. ? They forbid the keeping of any thread-local state. Especially in the Java EE environment, container threads (for example, from servlet thread pools) may call into DDS APIs. DDS state attached to these threads constitutes a resource leak that will prevent applications from being properly unloaded from the container. In other words, it is not possible for OMG-provided classes to know by themselves where to delegate their implementation. That knowledge must come from the application. Design Alternatives: There are two ways an application can provide the necessary implementation-specific context: • Indirectly: An (already-created) object can provide it to another object at the time of creation. This boils down to factory methods: if instance X can store implementation-specific state, and X is a factory for Y, then X can provide that state transparently to Y in a “createY()” method. Currently, the specification takes this approach where it is implied by the PIM. Alternatively, it could be extended everywhere: make the Bootstrap class (or another class) an explicit factory for every top-level type in the API. • Directly: The application can pass the calling environment as an argument. Currently, the specification takes this approach when accessing singletons or constructing instances of top-level classes. We can choose the right approach in each instance if we understand how Bootstrap is used today. Background: Bootstrap Occurrences: The class is used in the following places: 1. To access per-DDS-implementation singletons: DomainParticipantFactory and DynamicTypeFactory. 2. To create Entity-independent reference objects: WaitSet, GuardCondition, and TypeSupport. We could reduce the number of occurrences of Bootstrap by making accessors/factory methods for DynamicTypeFactory, WaitSet, GuardCondition, and TypeSupport available as instance methods of DomainParticipantFactory. 3. To create standalone value objects: Time, Duration, and InstanceHandle. These occurrences will be hard to eliminate. 4. To create instances of Status classes. We could eliminate these occurrences of Bootstrap by creating Status objects from factory instance methods on the corresponding Entity interfaces. 5. To create instances of built-in topic data types: ParticipantBuiltinTopicData, BuiltinTopicKey, etc. These occurrences will be hard to eliminate. 6. To access convenience sets of Status Class objects—the equivalent of STATUS_MASK_ALL and STATUS_MASK_NONE. We could eliminate these occurrences by making these accessors instance methods. [Rick] Proposal The fact that this issue came up indicates that the rationale for the Bootstrap class is not sufficiently clear. Add such an expanded rationale to the specification document. Furthermore, rename the class to ServiceEnvironment to make its role clearer. In addition, the method signatures for Bootstrap.createInstance() are incorrect for handling some OSGi deployment scenarios: it must be possible to supply the ClassLoader to be used to load the implementation class in order for dependencies to be resolved properly. Fix these signatures at the same time. With respect to the code, see the following revisions, which address the code changes: • Revision #139: http://code.google.com/p/datadistrib4j/source/detail?r=139 • Revision #151: http://code.google.com/p/datadistrib4j/source/detail?r=151 • Revision #166: http://code.google.com/p/datadistrib4j/source/detail?r=166 In the specification document, replace all occurrences of “getBootstrap” with “getEnvironment” and “Bootstrap” with “ServiceEnvironment”. These occurrences are in these sections: • 7.1.4, “Concurrency and Reentrancy” • 7.2.1, “ServiceEnvironment Class” (previously “Bootstrap Class”) • 7.3.1, “DomainParticipantFactory Interface” • 7.7.1.1, “DynamicTypeFactory Interface” At the end of section 7.2.1, “ServiceEnvironment Class” (previously “Bootstrap Class”), add the following paragraphs: Design Rationale (non-normative) This class is designed to avoid the brittle mixing of concrete implementation with abstract specification that would occur if either the specification mandated implementation or if vendors re-implemented different classes with the “same” names. In addition, it is designed to enable the following deployment scenarios: • Standalone deployment of a single application using one or more DDS implementations. (The expected use case for multiple implementations is a DDS-to-DDS bridge.) • Deployment within a Java EE or OSGi container, which may host multiple independent applications. More than one of application may use DDS internally, unknown to other applications. Each of these should be able to declare, “I depend on DDS” and allow the platform’s administrator to inject the implementation. The requirements above preclude a DDS vendor from reimplementing any OMG-provided type, and they preclude OMG-provided types from keeping any static or thread-local state. Resolution: The task force feels that this issue requires further discussion before it can be resolved. (It was filed after the comment deadline.) It is therefore deferred. Disposition: Deferred
Some of the DDS QoS Policies are Request vs. Offered in the sense that the value of matching policies on communicating entities have to satisfy a specific ordering relationship. Specifically, the policy set on the receiving side should always be less or equal than the analogous QoS Policy on the sending side. As a result there is a natural total ordering for RxO Policies which is not currently captured nor reflected in the API. As a consequence also QoS should be defining a total order. [Resolution] Ensure that all RxO Policies implement the Comparable interface. This is pretty logical as for this QoS Policies it has to be established a total order. Let QoS classes also implement a comparable interface.
[Angelo] Proposal: Ensure that all RxO Policies, and all entity QoS types, implement the Comparable interface.
[Rick] There is no total order over QoS types, because they consist of multiple QoS policies, which may not all share the same relative ordering.
Re: only QoS policies then, it is true that RxO contracts define an ordering of policies. But saying that a writer with a certain policy offers a “greater” level of service than a reader with another policy is a very different sort of comparison than saying that one integer is “greater” than another. Therefore, I am not completely comfortable with this issue, although I am interested to hear what others have to say.
Therefore, I propose that, rather than implementing the Comparable interface, these policy types should provide a Comparable object. This approach is functionally identical but clarifies which comparison contract is being used. For example:
if (dwPolicy.requestedOfferedContract().compare(drPolicy) >= 0) {
System.out.println("Compatible!");
}
See revision #155: http://code.google.com/p/datadistrib4j/source/detail?r=155, which contains an initial provisional expression of this resolution. See also revision #167: http://code.google.com/p/datadistrib4j/source/detail?r=167, which adds support for DDS-XTypes-derived policies.
QoS Policies define a nested ID class for capturing the Policy ID and PolicyName. [Resolution] Remove the ID class and (1) use Java introspection for accessing the policy name, and (2) define an integral ID for specifiying the policy ID. Notice that getid and getName methods are also needed.
Constant values such as the infinite duration, etc. should be defined by the standard as opposed than the implementation. [Resolution] Define constants as part of the API.
Infinite duration and other well-defined values are defined by the standard. The class defines an operation infiniteDuration: Duration. Furthermore, the documentation of the getDuration reads, “If this duration is infinite, this method shall return Long.MAX_VALUE….” Similar operations and documentation are given for invalid Time, nil InstanceHandle, and so on.
The dds-psm-java makes use of import as a way to take care of the @link directive on Javadoc. This is not a good practice and it is better to use the fully qualified type name on the @link javadoc directive [Resolution] Clean up all the spurious import and use fully qualified types on the @link directives.
See revision #132: http://code.google.com/p/datadistrib4j/source/detail?r=132. (This revision does not resolve this issue. It addresses only the JavaDoc package files.) Resolution: Eventually, use fully qualified types on the @link directives, removing any subsequently unnecessary import statements. However, this issue, which was filed after the comment deadline, does not impact the correctness, performance, compatibility, or user experience of the specification. Therefore, while there is a minor issue of “cleanliness”, resolving it is not a priority. Therefore, this issue is deferred. Disposition: Deferred
The absence of a DSL for facilitating the correct creation of QoS (in QoS classes such as:
TopicQos, DataWriterQos, etc.) in the
dds-psm-java not only makes QoS manipulation cumbersone, but it also
introduces potential for errors.
[Resolution]
Define a QoS DSL for the dds-psm-cxx which might look like this:
TopicQos topicQos =
(new TopicQos())
.with(Reliability.Reliable(), Durability.Transient());
This is also legal:
TopicQos topicQos =
(new TopicQos())
.with(Reliability.Reliable())
.with(Durability.Transient());
- These class should implement the Comparable interface as they need to
provide a total order... Otherwise how can one do RxO?
[Angelo] Proposal: Define a QoS DSL for the DDS-PSM-Java, which might look like this:
TopicQos topicQos =
(new TopicQos())
.with(Reliability.Reliable(), Durability.Transient());
This is also legal:
TopicQos topicQos =
(new TopicQos())
.with(Reliability.Reliable())
.with(Durability.Transient());
Resolution:
Defer this issue. It was filed after the comment deadline, and its resolution will be coupled to those of issues #15966 and #16529. Because of this complexity and the short time available, it is not possible to resolve it effectively at this time.
Disposition: Deferred
The Entity provides some generic methods that seem of doubtful usefulness but then on the other end open up a door for messing up with the invariant of a type or at least raising runtime errors. For instance via the Entity type I can add a non-applicable QoS policy to a DDS entity -- this seems weakening the API. [Resolution] Remove all method that might break invariants such as setQos, setListener, etc.
[Angelo] Proposal: Remove all method that might break invariants such as setQos, setListener, etc. [Rick] This issue is unclear to me. It does not indicate which methods it considers unimportant, nor does it indicate how type safety may be weakened.
What is the value of having the DomainEntity class? [Resolution] The DomainEntity class should be removed and the getParent method should be migrated to the Entity class.
[Angelo] The DomainEntity class should be removed and the getParent method should be migrated to the Entity class. The DomainEntity is not on any other PSM, thus I don't see why it should be on the Java PSM. Even at a PIM level it is a class w/o attributes and w/o operations—thus a very debatable abstraction.
The read API currently seems a bit too complicated. In some in some instances
it provides part of the results as a return value and the rest by means of arguments.
This does not feel right and again violates one of the key goal of having a new PSM: simplcity
and usability.
The API does not provide a way of deciding if one wants to read/take only valid
data. This is a remark true in general for DDS which needs to be fixed for all PSM
as well as for the PIM!
The following methods on the DataReader interface are superfluous:
- cast
- createSample
[Resolution]
Refactor and cleanup the data-reader API.
The org.omg.dds.core.status.Status class currently extends the java.util.EventObject.
The issue I have with this is that a status and an event are to different concepts.
A status represents a continuous value or set of values which are always defined,
while an event represents and happening. For instance an event could be used to notify
the change of status but not the status itself.
[Resolution]
That said the refactoring suggested is to re-organize the current status types
so to clearly distinguish what is are statuses and what are the events. As such,
all the status currently defined should remove reference to the source. Why?
Because the statuses are retrieved from the source thus it is kind of silly to
add back the source on the communication status.
Let me give you an example ("dr" below is a DataReader):
RequestedDeadlineMissedStatus s = dr.getRequestedDeadlineMissedStatus();
// this give back the reader we already know, thus it is not real useful
// information which should simply be removed.
s.source())
BTW the status types as well as the relative accessor methods should
drop the trailing "Status" as it is not so informative.
That said, we should add an event type associated to each status defined like this:
class RequestedDeadlineMissedEvent {
private RequestedDeadlineMissed status;
private DataReader source;
//... useful methods
}
The event type is the one that should be used as a parameter of the listener methods.
Finally, it is worth noticing that the suggested refactoring will fix the
DataAvailableStatus anomaly. This type currently defined as a status is actually
an event and as such should be treated. So where is the anomaly, for this status
there are no methods on the data reader and there is really no status information
such as saying... Yes there are 15 new samples or something like this.
The methods that access the communication statuses should simply return an object as opposed to also require it as an argument. As the Status is immutable there is no risk in the client code changing it, thus no copies required! [Resolution] Apply the changes suggested in the description above.
The dds-psm-java currently passes statuses via collections. However this does not make it neither efficient nor simple to represent collections of related statuses, such as the Read Status, etc. [Resolution] The suggestion is to add a ReadState as currently present on the dds-psm-cxx to simplify the API and make it simpler to support the most common use cases.
Description
----------------
The DDS-PSM-Java provides bucket accessors that allow to "return" an object
by "filling" a method parameter.
As an example, for a property Foo there would be a method:
Foo f = // some foo
x.getFoo(f)
The rationale for this API is to avoid a defensive copy of Foo each time it is accessed.
However the cost of this "optimization" is an API that has side-effects everywhere,
with all the nasty implications of side-effects.
Resolution
---------------
The solution suggested to avoid bucket accessors and thus side-effects is to
rely as much as possible on immutable objects (e.g. value-types). This ensures
that (1) defensive copies are unnecessary since the attribute returned is immutable,
and (2) new objects are created when new values are required.
If properly designed (as shown on an issue posted on QoS and Policies) this approach
not only leads to a simpler and safer API, but it also leads to actually save memory in most
of the cases.
The only case where the suggested approach has a cost is when a property changes
very often. However, in many of these cases (often found in loops) the new JDK7
escape analysis will help greatly help in dealing with the potential garbage as it
will allocate these short-lived objects on the stack.
[Angelo] Proposal: The solution suggested to avoid bucket accessors and thus side effects is to rely as much as possible on immutable objects (e.g. true value types). This ensures that (1) defensive copies are unnecessary since the attribute returned is immutable, and (2) new objects are created when new values are required. If properly designed (as shown on an issue posted on QoS and Policies) this approach not only leads to a simpler and safer API, but it also leads to actually save memory in most of the cases. The only case where the suggested approach has a cost is when a property changes very often. However, in many of these cases (often found in loops) the new JDK7 escape analysis will help greatly help in dealing with the potential garbage, as it will allocate these short-lived objects on the stack. [Rick] The other place there is a high cost of new allocations in on the critical read/take and write paths. I agree that the number of occurrences of this pattern can be reduced. But due to the complexity of choosing which ones to change, and the late submission date of this issue, I recommend deferring it. Resolution: Defer this issue. It was submitted after the comment deadline. Disposition: Deferred
The Sample Class as defined in the Beta1 of the DDS-PSM-Java does not define
a method "isValid(): Boolean" to check wether the data is actually valid. This is a simple
to fix oversight.
Resolution
---------------
Add an "isValid () : Boolean" method to the Sample Class
[Angelo] An implementation could decide to provide the last known value. Would be useful to separate the null from the strictly valid data.
The name of the classes DataReaderAdapter/DataWriterAdapter are misleading since what they are really providing are listeners with some default behaviour. Resolution --------------- Rename the classes DataReaderAdapter/DataWriterAdapter to SimpleDataReaderListener and SimpleDataWriterListener. For the SimpleDataReaderListener one could implement trivially all the method but the one that notifies the availability of data, e.g. <onDataAvailable>
[Angelo] Proposal: Rename the classes DataReaderAdapter/DataWriterAdapter to SimpleDataReaderListener and SimpleDataWriterListener. For the SimpleDataReaderListener one could implement trivially all the method but the one that notifies the availability of data, e.g. onDataAvailable.
ContentFiltered topics, QueryCondition, and MultiTopic all require a "Query" parameter made by an expression and a set of parameters. The current API, however treats the expression and the parameter as individual parameters and does not provide any abstraction of what could represent a generic DDS query. This makes the API more verbose and more error prone. Resolution --------------- Add a Query class that abstracts over the concept of a DDS query: an expression and a collection of mutable parameters
The base interface for all Entity-level QoS objects (e.g. DataReaderQos) is org.omg.dds.core.EntityQos. At one time during the evolution of the specification, this interface was called org.omg.dds.core.Qos. Due to an editorial oversight, this obsolete name persists in the specification document and should be updated. * Section 7.2.5, "QoS and QoS Policies" * Section 7.2.5.2, "Entity QoS"
DDS code will be easier to integrate into third-party I/O code if the Entity, ReadCondition, and TopicDescription interfaces implement the java.util.Closeable interface. This is especially true under Java 7, which provides specific new language constructs for dealing with this interface. The only method in the interface is a no-argument close(), which all of these interfaces already have.
The second FTF of the DDS-XTypes spec introduced several API changes that should be incorporated into the DDS-PSM-Java spec. At the same time, the contents of the relevant portions of the DDS-XTypes spec should be incorporated as JavaDoc comments, just as has already been done for DDS itself.
The EntityQos interface implements java.util.Map. However, all checking of which policies apply to which Entity types is deferred to run time. The extension of Map should be updated to constrain which policies may legally be used. Proposal: Introduce marker interfaces for each Entity type and parameterize Map with these interfaces.
See revision #185: https://code.google.com/p/datadistrib4j/source/detail?r=185 These changes are also available in file diff_omg_issue_17304.txt in ptc/2012-12-08 (issue_diffs.zip)
Java 7 has a try-with-resources construct that allows a close() method to be called automatically when a certain code block ends. Java PSM can support this construct with sample loans in a way that's backwards compatible with Java 5. All we have to do is to rename the Sample.Iterator.returnLoan() method to close() and make Sample.Iterator implement the interface java.io.Closeable. Note: java.lang.AutoCloseable is available only since 1.7
The dds-psm-java uses a superfluous "QoSPolicy" suffix to name the DDS policies which themselves are already included in a "policy" namespace. This issue is identical to issue #16530. This issue is created to revote on the decision taken on issue #16530 in the earlier FTF. Proposed Resolution: This suffix should be removed. This resolution will also make Java PSM consistent with the C++ PSM, which does not use "QosPolicy" suffix.