On 6/29/2015 10:35 AM, Sergey Bylokhov wrote:
Hi, Jim.
Thanks for review!
Thew new version of the fix:
http://cr.openjdk.java.net/~serb/7188942/webrev.02
Comments about pbuffer were changed.
CGLGC.java:
In createCompatVM() I dislike separated tests for "here is early
rejection of the list of things that I can handle" followed by a list
of tests for things it can handle. For one thing we have to test the
type more than once. But mainly it just seems like the two tests can
get out of synch over time. It just seems cleaner to only iterate a
set of allowed types once. Wouldn't it be simpler and more
straightforward to do something like:
The different ifs were merged. I think the code became more readable
after that.
It's shorter, and it's fine, but personally I prefer to spell things
out. In particular, it gets confusing that your expression has two
clauses that both test type ==/!= FB which causes me to make finger
pictures in the air to pull it apart. The cascading tests I wrote are
more straightforward to follow and you can see right away that BITMASK
is rejected regardless of type and that only FB and TEXTURE are allowed
and that FB has one additional test. But, that's just a question of
style - at least the new version consolidates the tests into a single
expression...
...jim
