David and Dmitry, thanks for the reviews!

dl

On 1/22/2015 11:41 PM, David Holmes wrote:
On 23/01/2015 5:36 PM, Dean Long wrote:
On 1/22/2015 11:01 PM, David Holmes wrote:


On 23/01/2015 4:01 AM, Dean Long wrote:
On 1/22/2015 2:19 AM, David Holmes wrote:
On 22/01/2015 8:39 AM, Dean Long wrote:
Thanks Dmitry.  The updated webrev is here:

     http://cr.openjdk.java.net/~dlong/8031064/webrev.3/

This looks weird:

+ VMDEF_PAT  = ^_ZTV
+ VMDEF_PAT := ^gHotSpotVM|$(VMDEF_PAT)
+ VMDEF_PAT := ^UseSharedSpaces$$|$(VMDEF_PAT)
+ VMDEF_PAT := ^_ZN9Arguments17SharedArchivePathE$$|$(VMDEF_PAT)

but I can sort of see why you wanted to do it that way.


Do you have a suggestion for a less-weird-looking way to do it?

Only the obvious but hard to read:

VMDEF_PAT :=
^_ZN9Arguments17SharedArchivePathE$$|^UseSharedSpaces$$|^gHotSpotVM|^_ZTV


Sure, I will do that.  Can I count that as "reviewed"?  And do I need
another Reviewer for this change, or am I good to go?

If you change it you will have to redo all your testing. Just leave as-is - the readability wins here. Up to you though.

Dmitry and I have reviewed this. So you have one "Reviewer" and one "reviewer" :)

Thanks,
David

dl

I assume you have verified the results are identical?


Yes.

I would be good to see this applied uniformly across all platforms as
well (except windows).


I suppose, but isn't linux the only platform where we might be
cross-compiling?  I'm not setup to
build for aix, bsd, or solaris, and if I build in JPRT, I'm not sure it
will save the vm.def or mapfile to
make a comparison against. Can we make cleaning up the other platforms
a separate RFE?

Getting rid of shell scripts from the build is a Good Thing(TM)! But
yes we can make this a separate RFE.

Thanks,
David


dl

Thanks,
David

dl

On 1/21/2015 12:11 AM, Dmitry Samersoff wrote:
Dean,

vm.make  ll. 247

1. *.o should be  $(Obj_Files)

2.

$(NM) --defined-only *.o | sort -k3 -u |
  awk '/$(VMDEF_PAT)/{ print "\t" $$3 ";" }'

should give you the same result with less efforts

-Dmitry

On 2015-01-21 07:59, Dean Long wrote:
Here's version 2, which does everything in vm.make and doesn't do
anything that is shell-specific:

http://cr.openjdk.java.net/~dlong/8031064//webrev.2/

thanks,

dl





Reply via email to