The records are norally terminated on the first unquoted '/', but for the UDQ
and ACTIONX keywords the data sections of a keyword contain mathematical
expressions which can contain '/' literals. This commit adds a per-keyword
ability to terminate the records on the last '/' instead of the first '/'.
Tune the makefile according to new principles, which adds a few bells
and whistles and for clarity.
Synopsis:
* The dependency on opm-common is completely gone. This is reflected in
travis and appveyor as well. No non-kitware cmake modules are used.
* Directories are flattened, quite a bit - source code is located in the
lib/ directory if it belongs to opm-parser, and external/ if third
party.
* The sibling build feature is implemented through cmake's
export(PACKAGE) rather than implicitly looking through source files.
* Targets explicitly set required public and private include
directories, compile options and definitions, which cmake will handle
and propagate
* opm-parser-config.cmake for downstream users is now provided.
* Dependencies are set up using targets. In the future, when cmake 3.x+
can be used, these should be either targets from newer Find modules,
or interface libraries.
* Fewer system specific assumptions are coded in, instead we assume
cmake or users set up system specific details.
* All module wide configuration and looking up libraries is handled in
the root makefile - all sub directories only set up libraries and
compile options for the module in question.
* Targets are defined and links handled transitively because cmake now
is told about them. ${module_LIBRARIES} variables are gone.
This is largely guided by the principles outlined in
https://rix0r.nl/blog/2015/08/13/cmake-guide/
Most source files are just moved - if they have some content change then
it's nothing more than include fixes or similar in order to make them
compile.
Just relying on the char data type is not sufficient to guard against
overflows, and several input decks would invoke undefined behaviour.
This code path is extremely hot, so we're essentially only reading the
least significant 7 bits to achieve branchless lookup.
The hand-written number parser functions implemented using strtod and
friends were rather slow (profiling indicates that typically 30% of the
program is spent inside of strtod internals). By using
boost::spirit::qi, which we already depend on through boost-filesystem
and others this portion typically seem to be reduced to 20% (via
instruction count) and with somewhat better cache performance.
Rudimentary measuring indicates ~15% speedup overall.
Additionally, the intention is a lot clearer this way, so readability
received a boost. Compilation time of StarToken goes through the roof.
Implementing these checks as function objects improves performance
slightly (5% or so according to my measurements), probably due to the
functions being inlined rather than reduced to function pointers.
Reuses the original records string_view rather than expanding to the
same std::string, we save some allocations, memory cache misses and
simplify the class slightly.
Additionally, the uninteresting add-multiple-identical-records logic
ParserItem did before has been moved into RawRecord and is now performed
by std::deque (which also means it can allocate better for itself). The
addition of prepend deprecates push_front.
When the TITLE keyword was present in the deck, but no parameter was
given the parser would consume the next keyword as the simulation TITLE.
Override this by writing a default TITLE if it's unset.
The splitting of RawRecords into individual symbols uses string_view.
Also updates tests since RawRecord now assumes that the record string it
receives is complete and does *not* contain the terminating slash.
Since string_view uses char* for representation, there is no longer a
need to copy into a local char array for most code paths (only when the
input must be modified due to fortran float formatting).
By representing string_view as char* instead of
std::string::const_iterator the string_view class bring possibly
slightly lower overhead, but mostly enables some optimisations.
Several inner parser functions modified to use string_view, to reduce
unecessary copying (and indirectly allocationg) related to passing
strings around.
When considering if a keyword is valid, the parser procedures convert
the same string to uppercase twice, with copies. This behaviour has been
changed, and ParserKeyword now assumes it will be given a
correctly-formatted keyword to look up.
The implementation has been rewritten to use iterators and renamed for
internal use. The public static function still exists for testability
and easy verification, but should be considered an internal part of the
parser.
The general loop through all raw records should be based on the iterator
interface of the RawKeyword, but to resolve INCLUDE statements we have
implemented a special case method to get the first record.
Changes the implementation of numbers parsing from std::atoi/f to
std::strtod/l. These support setting the optional end-of-string pointer
which are used to determine if a parsing was successful or not. This has
the nice side effect of *greatly* simplifying the logic, at the expense
of some C-style details.
Tests added to verify that the different edge cases are handled
properly.
Since RawRecords now has automatic storage, managed by std::list,
offering iterators is feasible. The random access
RawKeyword::getRecord's real use was accessing the records in order,
which now is handled via iterators.
By changing the underlying storage of RawKeyword to std::list, we ensure
that the RawRecords aren't reallocated and moved, preserving the
validity of string_view's. This changes the access complexity of
RawRecord from O(1) to O(n).
These functions are called a lot and are trivial accessors to the
underlying containers. By opening them for inlining we get a decent
performance benefit "for free" via optimisation opportunities.
The accumulated strings are moved into RawRecords, which reduces
execution time (rough measurements indicates 4-8%). To facilitate this,
RawRecords are stored directly in the vector in favour of via
shared_ptrs.
An attacker using very long decimal integers as input could trigger a
buffer overflow write during int/double parsing.
The vulnerability has been fixed and raw buffer boundaries are checked.
Additionally, integer buffer size is determined by platform 'int' width.
'double' uses a heuristic to support both pure decimal formats (up to 64
characters long) and float formats.
The push_front() method can cause reallocation of expanded_items,
thereby invalidating iterators already stored in m_recordItems.
Switching to std::list fixes this.
readValueToken spent almost half its time dealing with weirdly formed or
broken floats. Now has a shorter path that can early return a
successfully parsed float and only do slow handling of cases that need
it (notably zero, fortran style exponent and errors).
The boost provided lexical cast are inefficient and is shown to be a
slowdown in the inner loop. Replaces them with std::atoi/std::atof and
some simple correctness checking.
