Mailing List Archive: Perl: porters

Re: PATCH #69018; revamped mktables; Makefile changes for it

Index | Next | Previous | View Threaded

public at khwilliamson

Nov 22, 2009, 9:46 PM

Post #1 of 3 (263 views)
Permalink

Re: PATCH #69018; revamped mktables; Makefile changes for it

Rafael Garcia-Suarez wrote:
> 2009/11/22 karl williamson <public [at] khwilliamson>:
>> A revised mktables is available, both at
>> git://github.com/khwilliamson/perl.git. (The branch is called mktables) It
>> fixes the minor bug #69018, concerning accepting the erroneous
>> \p{Script=InGreek}, and perhaps other bugs; I need to look. But it fixes a
>> number of things which I have not bothered to write bug reports on, many of
>> them have been aired on the p5p list over the last several months.
>
> Thanks a lot. I've merged your patch in bleadperl. I'll update perldelta
> as well.
>
> I also checked in 3 small mundane commits on top of it.
> I noticed that mktables.lst was modified after build, so I removed it
> from the sources, letting the build process generate it at each
> time. Is that correct?
>
> commit 23f0c133b39568de35784182a730cb2929c27e34
> Author: Rafael Garcia-Suarez <rgs [at] consttype>
> Date: Sun Nov 22 21:37:51 2009 +0100
>
> Remove mktables.lst, since it's generated
>
> MANIFEST | 1 -
> lib/unicore/mktables.lst | 829 ----------------------------------------------
> 2 files changed, 0 insertions(+), 830 deletions(-)
>
> commit 329347eef534abb140be6ddc937e0ef89f630d3d
> Author: Rafael Garcia-Suarez <rgs [at] consttype>
> Date: Sun Nov 22 21:10:38 2009 +0100
>
> Add new generated file lib/unicore/Heavy.pl to gitignore
>
> Also remove from there files that are no longer generated by mktables.
>
> lib/.gitignore | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)

Is it documented somewhere that it should be done? Thanks for doing it.
>
> commit 3aa7f313a30095b1306546e95c9a127b52cc2888
> Author: Rafael Garcia-Suarez <rgs [at] consttype>
> Date: Sun Nov 22 19:50:33 2009 +0100
>
> Fix plan syntax in TAP output
>
> t/re/uniprops.t | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)

Sorry
>

There needs to be some discussion about how mktables gets called. It
generates all three of t/re/uniprops.t, lib/unicore/mktables.lst, and
pod/perluniprops.pod (besides Heavy.pl and all the tables). I meant to
patch Makefile to cause all three of these to be generated, but didn't
realize that I should really patch Makefile.SH instead, so my changes
were lost. But let's figure out what they should be before I submit a
revision.

These three files are essentially static from Unicode release to Unicode
release (as are all the tables, except if someone changes mktables
itself to generate different things.) The reason you saw mktables.lst
change is because it has a time stamp in it (that should have been the
only difference, and it was because I did a final make test, just to be
sure, after I had committed, but before I delivered; sorry).
mktables.lst retains the same format as it traditionally has had.

I was attempting to add the generation of all three of them to Makefile
because of what Nicolas said several months ago about wanting to
generate the pod file every time, as it was one less thing for the
pumpking to worry about when getting things out the door. I figured
that we should be consistent, so made that the behavior for all three.
But this is a change for mktables.lst. I don't know when that got
generated in the past. Perhaps it is something on the pumpking's list?

I see a couple of possibilities. One is to generate all three every
time. I guess that means they should come off the MANIFEST. But
mktables.lst has always been generated, and has been in MANIFEST.

The other option I see is to generate them by hand when a new release of
Unicode is installed, and to put them in the MANIFEST. But if someone
changes mktables to generate different files, these would have to be
regenerated at that time as well, and someone might forget.

What are people's opinions?

Somewhat related, this new mktables takes longer than the old one to
execute. In part it is because it's doing a lot more things; in part
it's because it uses more object-oriented, inside-out-hash, functions
and is getting them in pure Perl. (Nicholas' suggestion from last week
helped quite a bit here, though). Even on my this-year's-model Linux
box, it seems like it takes forever. (I compile my development Perls
without optimization; it seems like it runs twice as fast on a 5.10 Perl
that has optimization, but I haven't done any benchmarking.) Adding the
-p option to the call to show progress helps me see how things are
progressing, and realize how much work it's actually doing. Should I do
that in the Makefile?

I think this longer compilation time would be perfectly acceptable if it
didn't actually happen very often, like with the Encode tables. And
there is no reason to compile more frequently than the Encode tables.
As I mentioned, the underlying tables are static. New Unicode releases
come out about once a year or so. But Makefile is removing outputs
before testing whether it should call mktables, and of course, decides
it should. I haven't sat down to figure out how this is happening yet;
nor have I investigated when the Encode tables get cleaned; is it a
normal clean, I don't think so; maybe its a realclean. But I think
mktables' outputs shouldn't be cleaned at a lower level than Encode's.
Comments?

nick at ccl4

Nov 23, 2009, 1:51 AM

Post #2 of 3 (236 views)
Permalink

Re: PATCH #69018; revamped mktables; Makefile changes for it [In reply to]

On Sun, Nov 22, 2009 at 10:46:26PM -0700, karl williamson wrote:

> There needs to be some discussion about how mktables gets called. It
> generates all three of t/re/uniprops.t, lib/unicore/mktables.lst, and
> pod/perluniprops.pod (besides Heavy.pl and all the tables). I meant to

> I was attempting to add the generation of all three of them to Makefile
> because of what Nicolas said several months ago about wanting to
> generate the pod file every time, as it was one less thing for the
> pumpking to worry about when getting things out the door. I figured
> that we should be consistent, so made that the behavior for all three.
> But this is a change for mktables.lst. I don't know when that got
> generated in the past. Perhaps it is something on the pumpking's list?
>
> I see a couple of possibilities. One is to generate all three every
> time. I guess that means they should come off the MANIFEST. But
> mktables.lst has always been generated, and has been in MANIFEST.
>
> The other option I see is to generate them by hand when a new release of
> Unicode is installed, and to put them in the MANIFEST. But if someone
> changes mktables to generate different files, these would have to be
> regenerated at that time as well, and someone might forget.
>
> What are people's opinions?

Tricky.

The logic to run tests

1: needs to be simple, because it needs to work well enough if perl has not
built correctly
2: currently relies on getting a list of tests to run from MANIFEST
(which is consistent with point 1 - it is KISS)

The simplest way to sit within those constraints is to have t/re/uniprops.t
as a wrapper which requires a generated file. (No, this isn't totally clean.
But I can't see a cleaner way to do it)

I also continue to feel strongly that it is wrong ship generated files if we
can avoid it. Shipping generated files

1: pessimises the size of the release
2: increases work, and chance of mistake, on people maintaining the core.
in particular, it adds items to the checklist of things to do before a
release.

Given full information about dependencies, make is a very good tool at knowing
what *doesn't* need doing. If the current setup is needlessly regenerating
things, then that's the avenue to fix first.

Also, you mentioned Encode and the files it generated. I found a lot of scope
for optimisation within enc2xs, which dramatically decreased its memory use
and run time, without needing large fundamental design changes to how it
worked. And I managed that with only Devel::DProf. I'm curious what can be
achieved with Devel::NYTProf on mktables. (Which is a task that is within
the skill set of any of the 600 subscribers to this list. I'm hoping that it
might appeal to at least one.)

Nicholas Clark

rgs at consttype

Nov 23, 2009, 2:01 AM

Post #3 of 3 (240 views)
Permalink

Re: PATCH #69018; revamped mktables; Makefile changes for it [In reply to]

2009/11/23 karl williamson <public [at] khwilliamson>:
> There needs to be some discussion about how mktables gets called. �It
> generates all three of t/re/uniprops.t, lib/unicore/mktables.lst, and
> pod/perluniprops.pod (besides Heavy.pl and all the tables). �I meant to
> patch Makefile to cause all three of these to be generated, but didn't
> realize that I should really patch Makefile.SH instead, so my changes were
> lost. �But let's figure out what they should be before I submit a revision.

As far as I can tell it will only generate perluniprops.pod when passed
"-P pod", and t/re/uniprops.t when passed "-T t/re/uniprops.t".

> These three files are essentially static from Unicode release to Unicode
> release (as are all the tables, except if someone changes mktables itself to
> generate different things.) �The reason you saw mktables.lst change is
> because it has a time stamp in it (that should have been the only
> difference, and it was because I did a final make test, just to be sure,
> after I had committed, but before I delivered; sorry). mktables.lst retains
> the same format as it traditionally has had.

No, I saw other changes, notably in the list of output files, since by
default it doesn't generate the two files I just mentioned.

> I was attempting to add the generation of all three of them to Makefile
> because of what Nicolas said several months ago about wanting to generate
> the pod file every time, as it was one less thing for the pumpking to worry
> about when getting things out the door. �I figured that we should be
> consistent, so made that the behavior for all three. But this is a change
> for mktables.lst. �I don't know when that got generated in the past. Perhaps
> it is something on the pumpking's list?

I would favor generating everything by default, and making uniprops.t
be, as Nicholas said, a wrapper around a generated file (so uniprops.t
is listed in the MANIFEST and is picked up by the test harness.)

I would also make -p the default (show progress of mktables during run),
except maybe when stdout isn't a tty.

> I see a couple of possibilities. �One is to generate all three every time.
> �I guess that means they should come off the MANIFEST. �But mktables.lst has
> always been generated, and has been in MANIFEST.
>
> The other option I see is to generate them by hand when a new release of
> Unicode is installed, and to put them in the MANIFEST. �But if someone
> changes mktables to generate different files, these would have to be
> regenerated at that time as well, and someone might forget.
>
> What are people's opinions?
>
> Somewhat related, this new mktables takes longer than the old one to
> execute. �In part it is because it's doing a lot more things; in part it's
> because it uses more object-oriented, inside-out-hash, functions and is
> getting them in pure Perl. �(Nicholas' suggestion from last week helped
> quite a bit here, though). �Even on my this-year's-model Linux box, it seems
> like it takes forever. �(I compile my development Perls without
> optimization; it seems like it runs twice as fast on a 5.10 Perl that has
> optimization, but I haven't done any benchmarking.) �Adding the -p option to
> the call to show progress helps me see how things are progressing, and
> realize how much work it's actually doing. �Should I do that in the
> Makefile?
>
> I think this longer compilation time would be perfectly acceptable if it
> didn't actually happen very often, like with the Encode tables. �And there
> is no reason to compile more frequently than the Encode tables. As I
> mentioned, the underlying tables are static. �New Unicode releases come out
> about once a year or so. �But Makefile is removing outputs before testing
> whether it should call mktables, and of course, decides it should. �I
> haven't sat down to figure out how this is happening yet; nor have I
> investigated when the Encode tables get cleaned; is it a normal clean, I
> don't think so; maybe its a realclean. �But I think mktables' outputs
> shouldn't be cleaned at a lower level than Encode's. Comments?
>
>

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads