|
Cleaned up option handling. This includes using finals, making member variables
private whenever possible, and some formatting.
- fixed a bug in the string formatting for the double range validator
- OptionValidator, OptionValue, and their implementations now conspire not to
allow the creation of malformed options because the OptionType has been added
to validator calls to handle OptionValues that are created on demand.
Started with updated byte code rewrite from Jacques
Fixed several problems with scalar value replacement:
- use consistent ASM api version throughout
- stop using deprecated ASM methods (actually causes bugs)
- visitMethodInsn()
- added a couple of missing super.visitEnd()s
- fixed a couple of minor FindBugs issues
- accounted for required stack size increases when replacing holders for
longs and doubles
- added accounting for frame offsets to cope with long and double local
variables and value holder members
- fixed a few minor bugs found with FindBugs
- stop using carrotlabs' hash map lget() method on shared constant data
- fixed an incorrect use of DUP2 on objectrefs when copying long or double
holder members into locals
- fixed a problem with redundant POP instructions left behind after replacement
- fixed a problem with incorrect DUPs in multiple assignment statements
- fixed a problem with DUP_X1 replacement when handling constants in multiple
assignment statements
- fixed a problem with non-replaced holder member post-decrements
- don't replace holders passed to static functions as "out" parameters
(common with Accessors on repeated value vectors)
- increased the maximum required stack size when transferring holder members to
locals
- changed the code generation block type mappings for constants for external
sorts
- fixed problems handling constant and non-constant member variables in
operator classes
- in general, if a holder is assigned to or from an operator member variable,
it can't be replaced (at least not until we replace those as well)
- Use a derived ASM Analyzer (MethodAnalyzer) and Frame
(AssignmentTrackingFrame) in order to establish relationships between
assignments of holders through chains of local variables. This effectively
back-propagates non-replaceability attributes so that if a holder variable
that can't be replaced is assigned to from another holder variable, that
second one cannot be replaced either, and so on through longer chains of
assignments.
- code for dumping generated source code
- MergeAdapter dumps before and after results of scalar replacement
(if it's on)
- fixed some problems in ReplacingBasicValue by replacing HashSet with
IdentityHashMap
- made loggers private
- added a retry strategy for scalar replacement
if a scalar replacement code rewriting fails, then this will try to
regenerate the bytecode again without the scalar replacement.
- bytecode verification is always on now (required for the retry strategy)
- use system option to determine whether scalar replacement should be used
- default option: if scalar replacement fails, retry without it
- force replacement on or off
- unit tests for the retry strategy are based on a single known failure case,
covered by DRILL-2326.
- add tests TestConvertFunctions to test the three scalar replacement options
for the failing test case (testVarCharReturnTripConvertLogical)
- made it possible to set a SYSTEM option as a java property in Drillbit
- added a command line argument to force scalar replacement to be on during
testing in the rootmost pom.xml
In the course of this, added increased checking of intermediate stages of code
rewriting, as well as logging of classes that cause failures.
- work around a bug in ASM's CheckClassAdapter that doesn't allow for checking
of inner classes
Added comments, tidied up formatting, and added "final" in a number of places.
Signed-off-by: vkorukanti <venki.korukanti@gmail.com>
|
