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 16529: Modifiable Types should be removed and replaced by values (e.g. immutable types).
Issue 16531: Getting rid of the Bootstrap Object
Issue 16535: Large Number of Spurious Import
Issue 16536: QoS DSL Needed
Issue 16587: API Should Avoid Side-Effects, e.g. Remove Bucket Accessors
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 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 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
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
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
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.
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.