Feature #7126
replace 'new unknown()' with 'unknown.INSTANCE' in FWD conversion and runtime
0%
History
#1 Updated by Constantin Asofiei about 1 year ago
new unknown()
was replaced with unknown.INSTANCE
in parts of FWD runtime which showed up in profiling a large customer application. We need to make this more general:
- for conversion, any
new unknown()
must be converted tounknown.INSTANCE
- for runtime, replace all
new unknown()
occurrences withunknown.INSTANCE
As an unknown
instance is immutable, there is no reason to create new instances of it.
#2 Updated by Greg Shah about 1 year ago
Good idea. My only critique is that I want to find a different name than unknown.INSTANCE
. I think this is a bit confusing.
How about if we add import static com.goldencode.p2j.util.unknown.UNKNOWN;
and then simply reference UNKNOWN
? We could also call it UNKNOWN_VALUE
but I'd prefer the shorter text.
#3 Updated by Constantin Asofiei about 1 year ago
Greg Shah wrote:
How about if we add
import static com.goldencode.p2j.util.unknown.UNKNOWN;
and then simply referenceUNKNOWN
?
That works, too. I already have this unknown.INSTANCE
name in 7026a, I'll rename it in 7026a. Btw, I'm not planing to work on this task at this time, as the other new unknown()
references did not show in the profiler.
#4 Updated by Constantin Asofiei about 1 year ago
Renamed unknown.INSTANCE
to unknown.UNKNOWN
in 7026a/14493.
#5 Updated by Vladimir Tsichevski about 1 year ago
I can take this task.
A question: currently there is 3 ways to obtain the unknown
value:
- calling
new unknown()
, we want to get rid of this method, OK. To make sure, just make this constructor private. - referencing the
unknown.UNKNOWN
constant. I gather, this is the recommended method. - calling the
unknown.instantiateUnknown()
, this function just returns theunknown.UNKNOWN
constant. This method is called 8 times in the package, never referenced as a lambda.
I think, the last method should be eliminated either?
#6 Updated by Constantin Asofiei about 1 year ago
Vladimir Tsichevski wrote:
I can take this task.
Greg?
- calling
new unknown()
, we want to get rid of this method, OK. To make sure, just make this constructor private.
Correct, this will enforce the constant usage.
- referencing the
unknown.UNKNOWN
constant. I gather, this is the recommended method.
Correct.
- calling the
unknown.instantiateUnknown()
, this function just returns theunknown.UNKNOWN
constant. This method is called 8 times in the package, never referenced as a lambda.
This needs to remain, this is an impl for the abstract method in BDT.
Also, all "unknown" references in the TRPL code need to be reviewed and replaced with unknown.UNKNOWN
instead of new unknown()
.
#7 Updated by Vladimir Tsichevski about 1 year ago
Constantin Asofiei wrote:
- calling the
unknown.instantiateUnknown()
, this function just returns theunknown.UNKNOWN
constant. This method is called 8 times in the package, never referenced as a lambda.This needs to remain, this is an impl for the abstract method in BDT.
Now I see it, I have not noticed this due to the missing @Override
annotation :-(
#8 Updated by Greg Shah about 1 year ago
This task isn't a priority right now.