PATCH #69018; revamped mktables
Karl Williamson
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.
SKIP: personal_narrative {
This is close to a complete rewrite of mktables. I did not set out to
do this, but as the work progressed, I discovered more and more things
wrong. Having really looked into the Unicode history now, it appears to
me that when the original mktables was written, it was not clear what
direction Unicode would go in, and it went in a different direction than
anticipated. It took me quite a while to understand the distinction
between some data structures that were muddled. After that, the code I
was writing got clearer. I learned a lot about Unicode; at some point I
came to the shocking realization, when wondering why in tarnation did
the code do that?, that I had come to know more about the Unicode
standard than some of the patchers did. The old version's tables are
mostly correct, but there are a number of problems with them, some
subtle, some not so. The combining class table is so wrong that it
could easily be the butt of jokes; I've thought of a few myself.
Another problem was that many of the newer Unicode tables are unreadable
by the old mktables without extensive munging of them.
} end SKIP
SKIP: design goals {
I eventually gave up trying to fit into the existing mktables, and just
rewrote things. I've tried to make this work on auto pilot, so that new
Unicode releases will require a minimum of fuss. pod and .t files are
generated from the data, so that other things don't have to be patched
to keep up. I've also added much more input validation, so that if a
new enum value is added to a field, we will know, instead of blindly
ignoring it. There are more goals, but it's getting late, and hard for
me to think.
} end SKIP
The last part of this email includes text about the changes, intended
for perldelta. I'm not sure who's supposed to patch that. In addition
to those, here are the other things that have changed, but aren't
notable enough to mention externally. Hence, the most important changes
are after the line of ### in this email.
I've change the main Makefile to call this differently (besides the
parameters telling it where to put the pod and test files). mktables
need not actually run very often. The inputs are pretty static, just
like Encode's. And the dependency is on far more files than Makefile
knows about. This patch fixes the bug in mktables wherein it wrongly
calculated whether it should run or not; so I've removed the -w option
to it. When called that way, mktables will check and do nothing,
quickly, if nothing is out-of-date. If people don't trust that, we
could change it so the apparent critical dependencies are still known to
Makefile, and to use the -w option to force mktables to run when those
dependencies trigger it, and then to have an unconditional call to
mktables as well, without the -w, so that it can check its own
dependency list. That said, mktables is currently running too often;
there is something in Makefile that is removing some of these output
files when I don't think it should; I haven't had a chance to
investigate this.
The files that are generated for case mapping and folding continue to
have two parts, the regular part and a hash for special cases. It turns
out that a number of the special case entries could be handled just as
well using the regular method; so they have been moved there.
All duplicate files have been eliminated. That means that if two
properties match the same exact set of code points, one file serves
both. This was not so much to save disk space, as to save memory, as
the same swash can now serve multiple properties.
There are new options to mktables:
-globlist is used to attempt to process all .txt files in the
directory structure. The ones it doesn't know how to handle are
processed assuming that they follow the typical .txt syntax.
-P dir tells mktables to create perluniprops.pod in dir. Makefile
has been changed so this goes in the standard pod directory.
-T path tells mktables to create a .t file as 'path'. Makefile has
been changed so this goes into t/re/uniprops.t
-p tells mktables to give progress information as it works.
-c tells mktables to not output range counts in the .pl files
it generates. These are by default output as comments; I have found
them helpful for debugging, and they don't add much disk space.
Canonical.pl and Exact.pl have been replaced by Heavy.pl, which allows
for more straightforward code in utf8_heavy.pl
There are several new features which lay the groundwork for fixing
charnames to know about all code points and named sequences; \X to match
more correctly; and to allow other tables such as To/Digit.pl to be read
by the Perl core.
I removed a test from re/pat_advanced which relied on the old erroneous
definition of \w which included superscripts as part of a word; and
changed another test in regexp_unicode_prop.t for again a changed
property definition.
I have compared the outputs of this version and the previous and am
confident that all the differences are correct.
I tried to be scrupulous about using File::Spec, but tested this only on
Linux and Windows boxes, so there may be mistakes that should be smoked out.
I tried running perlcritic on this, but it crashed, apparently at an
innocuous place. I did mostly use the Perl Best Practices.
The rest of this is text intended to be suitable for perldelta. NOTE
that this includes some anticipated documentation changes that haven't
been submitted yet.
######################################################
Perl can now handle every Unicode character property. A new pod,
perluniprops, lists all available non-Unihan character properties. By
default the Unihan properties and certain others (deprecated and Unicode
internal-only ones) are not exposed. See below for more details on
these; there is also a section in the pod listing them, and why they are
not exposed.
Perl now fully supports the Unicode compound-style of using '=' and ':'
in writing regular expressions: \p{property=value} and
\p{property:value} (both of which mean the same thing).
Perl now supports fully the Unicode loose matching rules for text
between the braces in \p{...} constructs. In addition, Perl also allows
underscores between digits of numbers
All the Unicode-defined synonyms for properties and property values are
now accepted.
\p{...} matches using the Canonical_Combining_Class property were
completely broken in previous Perls. This is now fixed.
In previous Perls, the Unicode Decomposition_Type=Compat property and a
Perl extension had the same name, which led to neither matching all the
correct values (with more than 100 mistakes in one, and several thousand
in the other). The Perl extension has now been renamed to be
Decomposition_Type=Noncanonical (short: dt=noncanon). It has the same
meaning as was previously intended, namely the union of all the
non-canonical Decomposition types, with Unicode Compat being just one of
those.
\p{Uppercase} and \p{Lowercase} have been brought into line with the
Unicode definitions. This means they each match a few more characters
than previously.
\p{Cntrl} now matches the same characters as \p{Control}. This means it
no longer will match Private Use (gc=co), Surrogates (gc=cs), nor Format
(gc=cf) code points. The Format code points represent the biggest
possible problem. All but 36 of them are either officially deprecated
or strongly discouraged from being used. Of those 36, likely the most
widely used are the soft hyphen (U+00AD), and BOM, ZWSP, ZWNJ, WJ, and
similar, plus Bi-directional controls.
\p{Alpha} now matches the same characters as \p{Alphabetic}. The Perl
definition included a number of things that aren't really alpha (all
marks), while omitting many that were. The Unicode definition is
clearly better, so we are switching to it. As a direct consequence, the
definitions of \p{Alnum} and \p{Word} which depend on Alpha also change.
\p{Word} also now doesn't match certain characters it wasn't supposed
to, such as fractions.
\p{Print} no longer matches the line control characters: tab, lf, cr,
ff, vt, and nel. This brings it in line with the documentation.
\p{Decomposition_Type=Canonical} now includes the Hangul syllables
The Numeric type property has been extended to include the Unihan
characters.
There is a new Perl extension, the 'Present_In', or simply 'In'
property. This is an extension of the Unicode Age property, but
\p{In=5.0} matches any code point whose usage has been determined as of
Unicode version 5.0. The \p{Age=5.0} only matches code points added in 5.0.
A number of properties did not have the correct values for unassigned
code points. This is now fixed. The affected properties are
Bidi_Class, East_Asian_Width, Joining_Type, Decomposition_Type,
Hangul_Syllable_Type, Numeric_Type, and Line_Break.
The Default_Ignorable_Code_Point, ID_Continue, and ID_Start properties
have been updated to their current definitions.
Certain properties that are supposed to be Unicode internal-only were
erroneously exposed by previous Perls. Use of these in regular
expressions will now generate a deprecated warning message, if those
warnings are enabled. The properties are: Other_Alphabetic,
Other_Default_Ignorable_Code_Point, Other_Grapheme_Extend,
Other_ID_Continue, Other_ID_Start, Other_Lowercase, Other_Math, and
Other_Uppercase.
An installation can now fairly easily change Perl to operate on any
Unicode release. Perl is shipped with the latest official release, but
an installation can now download any prior release, and Perl will work
with that. Instructions are in perlunicode.pod
An installation can now fairly easily change which Unicode properties
Perl understands. As mentioned above, certain properties are by default
turned off. These include all the Unihan properties (which should be
accessible via the CPAN module Unicode::Unihan) and any deprecated or
Unicode internal-only property that Perl has never exposed.
The files in the To directory are now more clearly marked as being
stable, directly usable by applications. New hash entries in them give
the format of the normal entries which allows for easier machine
parsing. Perl can generate files in this directory for any property,
though most are suppressed. An installation can choose to change which
get written. Instructions are in perluniprops.pod
h...@crypt.org
:A revised mktables is available, both at
: 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.
Awesome work, I'm so glad somebody has hands tough enough to grasp these
thorns.
I don't know this stuff in any depth, but a few general things struck
me while reading.
:All duplicate files have been eliminated. That means that if two
:properties match the same exact set of code points, one file serves
:both. This was not so much to save disk space, as to save memory, as
:the same swash can now serve multiple properties.
Is this dealt with automatically, so that if two such properties later
diverge it will correctly split them out to separate files again?
: -p tells mktables to give progress information as it works.
If this is most useful for humans, it might be slightly nicer to make it
on by default, and instead add a -q[uiet] option to suppress it.
:The rest of this is text intended to be suitable for perldelta. NOTE
:that this includes some anticipated documentation changes that haven't
:been submitted yet.
The perldelta text mentions lots of effects on properties, but doesn't
say anything about things like \w and \d. If this patch affects what
they match then it is vital to mention that; but if it doesn't, that's
also important information that readers will be searching for.
Hugo
Ricardo Signes
> [ epic poem about unicode snipped ]
Holy cow, that looks like an absurd amount of work, and totally awesome.
Thanks for doing it!
--
rjbs
Rafael Garcia-Suarez
> 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 <r...@consttype.org>
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 <r...@consttype.org>
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(-)
commit 3aa7f313a30095b1306546e95c9a127b52cc2888
Author: Rafael Garcia-Suarez <r...@consttype.org>
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(-)
Karl Williamson
> karl williamson <pub...@khwilliamson.com> wrote:
> :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.
>
> Awesome work, I'm so glad somebody has hands tough enough to grasp these
> thorns.
>
> I don't know this stuff in any depth, but a few general things struck
> me while reading.
>
> :All duplicate files have been eliminated. That means that if two
> :properties match the same exact set of code points, one file serves
> :both. This was not so much to save disk space, as to save memory, as
> :the same swash can now serve multiple properties.
>
> Is this dealt with automatically, so that if two such properties later
> diverge it will correctly split them out to separate files again?
>
It tries to do everything it can automatically. And yes, it will
correctly split them out to separate files if they no longer match
identically.
> : -p tells mktables to give progress information as it works.
>
> If this is most useful for humans, it might be slightly nicer to make it
> on by default, and instead add a -q[uiet] option to suppress it.
There are actually several levels of verbosity, and -q changes it to
quiet: only output errors. The normal level outputs things that are
concerning but not necessarily errors. -p adds progress, and -v adds
even more stuff; I left it the essentially the same as it always has
been. It was a last minute decision on my part to not put in the -p. as
the default. I would have done this by adding it to the Makefile
parameters calling mktables. I thought people might not want this extra
information. We need to figure out how to call this program from the
Makefile; see my response to Rafael to follow shortly with more about this.
>
> :The rest of this is text intended to be suitable for perldelta. NOTE
> :that this includes some anticipated documentation changes that haven't
> :been submitted yet.
>
> The perldelta text mentions lots of effects on properties, but doesn't
> say anything about things like \w and \d. If this patch affects what
> they match then it is vital to mention that; but if it doesn't, that's
> also important information that readers will be searching for.
It hasn't been fully decided what's up with \w, \d, and \s in this next
release, and when it is, me or someone should patch perldelta to say
that. I'm adding it to my list of things to check. Thanks
>
> Hugo
>
Karl Williamson
Is it documented somewhere that it should be done? Thanks for doing it.
>
> commit 3aa7f313a30095b1306546e95c9a127b52cc2888
> Author: Rafael Garcia-Suarez <r...@consttype.org>
> 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?
Nicholas Clark
> 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
Rafael Garcia-Suarez
> 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.
Karl Williamson
turns out the problem was a combination of both Makefile and mktables.
I didn't realize that the files I removed were listed as dependencies in
Makefile, so it always thought it was out-of-date, and called mktables.
Part of the old code I hadn't changed caused mktables to remove all the
files when called with the -makelist option. This code is no longer
necessary, so I removed it.
I also changed Makefile to always call mktables, to let it decide if it
actually should recompile. I just commented it out for easy reversal.
The problem is that mktables depends on almost 800 files, located in the
To and lib subdirectories, and Makefile doesn't know about them all; and
they change from Unicode release to release. I'm assuming mktables.lst
was created because of the issues of trying to get them all known by
Makefile.
I did a search for the removed files and removed the only other
reference to them (in .gitignore); except I see them in Makefiles for
other platforms. I assume these get automatically generated from the
changed Makefile.SH
I also changed some typos in comments, rephrased a little of the
documentation, removed a no-longer-used subroutine, and fixed another
bug, in which it would fail to run if there was no mktables.lst at all,
added -p option to its call.
I'll work soon on a wrapper to the .t. I'm wondering though about the
generated pod. It should be put into the toc, etc. Would a zero-length
place-holder that mktables overwrites be the right solution?
Rafael Garcia-Suarez
> Attached is a patch to cut down the frequency of mktables running. It turns
> out the problem was a combination of both Makefile and mktables. I didn't
> realize that the files I removed were listed as dependencies in Makefile, so
> it always thought it was out-of-date, and called mktables. Part of the old
> code I hadn't changed caused mktables to remove all the files when called
> with the -makelist option. This code is no longer necessary, so I removed
> it.
Thanks, I've applied it now.
> I also changed Makefile to always call mktables, to let it decide if it
> actually should recompile. I just commented it out for easy reversal. The
> problem is that mktables depends on almost 800 files, located in the To and
> lib subdirectories, and Makefile doesn't know about them all; and they
> change from Unicode release to release. I'm assuming mktables.lst was
> created because of the issues of trying to get them all known by Makefile.
>
> I did a search for the removed files and removed the only other reference to
> them (in .gitignore); except I see them in Makefiles for other platforms. I
> assume these get automatically generated from the changed Makefile.SH
Well, no : they will need adjustment too. But I don't have Windows or
VMS machines or knowledge to test.
> I also changed some typos in comments, rephrased a little of the
> documentation, removed a no-longer-used subroutine, and fixed another bug,
> in which it would fail to run if there was no mktables.lst at all, added -p
> option to its call.
>
> I'll work soon on a wrapper to the .t. I'm wondering though about the
> generated pod. It should be put into the toc, etc. Would a zero-length
> place-holder that mktables overwrites be the right solution?
That's a good point. I added it already in pod.lst. I don't think that
a placeholder is necessary for the pod file. I'll have a look at it.
Rafael Garcia-Suarez
>> I'll work soon on a wrapper to the .t. I'm wondering though about the
>> generated pod. It should be put into the toc, etc. Would a zero-length
>> place-holder that mktables overwrites be the right solution?
>
> That's a good point. I added it already in pod.lst. I don't think that
> a placeholder is necessary for the pod file. I'll have a look at it.
Done now :
http://perl5.git.perl.org/perl.git/commitdiff/524ce141dcd46f87e73a300e1436937336261b19
Also rebuilt makefiles with pod/buildtoc (that rebuilds only the
pod-related part of makefiles.)
Steve Hay
> 2009/11/24 karl williamson <pub...@khwilliamson.com>:
>> I did a search for the removed files and removed the only other
>> reference to them (in .gitignore); except I see them in Makefiles for
>> other platforms. I assume these get automatically generated from the
>> changed Makefile.SH
>
> Well, no : they will need adjustment too. But I don't have Windows or
> VMS machines or knowledge to test.
>
Fixed for Win32 by 36ff7f95732aeec6ca1f7152b75dbedf5ce669d1
Rafael Garcia-Suarez
More fixes for UNIX and Win32 (to be checked) :
http://perl5.git.perl.org/perl.git/commitdiff/58fa074c95937d22a584fe789986e618c9fec5ff
I suspect VMS might be broken, too.
Karl Williamson
>> [...]
>
> 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.)
So, I tried it with NYTProf. As I expected (I had used DProf earlier),
the highest usage subroutine was my pure Perl version of
Scalar::Util::refaddr, reproduced below. A third of the total time was
spent in this routine. (This is required because miniperl doesn't do
dynamic loading, so refaddr is not available.)
When I was writing mktables, I was under the impression that refaddr
would be brought into the core for 5.12. There was an agreement to that
effect, but I guess no one ever got around to actually doing it.
When I run this under perl instead of miniperl, and change objaddr to
just return refaddr, the combination still takes quite a lot of time.
If refaddr were in the core would it be in-lined?
I found a few surprises; I haven't pored over the results, though. One
is that I left in a trace statement that got to the trace subroutine
before discovering that it had nothing to do. This added not very much
time.
Based on looking at existing code in utf8_heavy.pl, I had presumed that
the Perl optimizer would remove code that depended on a constant
subroutine that returns false. That is, 'foo if DEBUG' would be
optimized away if there was a line: 'sub Debug { 0 }' But that appears
to not be the case.
There were more string evals than I expected, though the total time did
not add up to all that much. I couldn't find a way in NYTProf to
highlight those.
There are two columns in the nytprofhtml output for subroutines that I
can't figure out what they mean, and saw no documentation for, 'P' and 'F'.
I also did not see anything there for memory usage. I don't know how
Perl handles using up too much memory. The old mktables kept all its
tables in memory, and so I felt free to do so as well. But the new
mktables handles quite a few more tables than the old one. I would
think you would get thrashing if the memory usage got too big. I wonder
if Steve's machine doesn't have much memory. When I run mktables using
perl instead of miniperl, I get execution times between 30 and 40 seconds.
Karl Williamson
# Returns the address of the blessed input object. Uses the XS
version if
# available. It doesn't check for blessedness because that would do a
# string eval every call, and the program is structured so that this is
# never called for a non-blessed object.
return Scalar::Util::refaddr($_[0]) if $has_fast_scalar_util;
# Get the package
my $pkg = ref($_[0]) or return undef;
# Change to a fake package to defeat any overloading
bless $_[0], 'main::Fake';
# Numifying a ref gives its address.
my $addr = 0 + $_[0];
# Return to original class
bless $_[0], $pkg;
return $addr;
}
I found that any overload in a class caused the numifying to fail if I
did it in that class; hence the blesses are necessary.
Aristotle Pagaltzis
> As I expected (I had used DProf earlier), the highest usage
> subroutine was my pure Perl version of Scalar::Util::refaddr,
> reproduced below. A third of the total time was spent in this
> routine. (This is required because miniperl doesn't do dynamic
> loading, so refaddr is not available.)
There was consensus for putting the Scalar::Utils stuff in core
a while ago… I wonder if that’s still on anyone’s radar.
Regards,
--
Aristotle Pagaltzis // <http://plasmasturm.org/>
Eirik Berg Hanssen
> Based on looking at existing code in utf8_heavy.pl, I had presumed
> that the Perl optimizer would remove code that depended on a constant
> subroutine that returns false. That is, 'foo if DEBUG' would be
> optimized away if there was a line: 'sub Debug { 0 }' But that
> appears to not be the case.
It needs a prototype: 'sub Debug () { 0 }'.
From perlsub:
# Functions with a prototype of "()" are potential candidates for
# inlining. If the result after optimization and constant folding is
# either a constant or a lexically‐scoped scalar which has no other
# references, then it will be used in place of function calls made
# without "&". Calls made using "&" are never inlined. (See
# constant.pm for an easy way to declare most constants.)
Eirik
--
All bridge hands are equally likely, but some are more equally likely
than others.
-- Alan Truscott
Karl Williamson
> karl williamson <pub...@khwilliamson.com> writes:
>
>> Based on looking at existing code in utf8_heavy.pl, I had presumed
>> that the Perl optimizer would remove code that depended on a constant
>> subroutine that returns false. That is, 'foo if DEBUG' would be
>> optimized away if there was a line: 'sub Debug { 0 }' But that
>> appears to not be the case.
>
> It needs a prototype: 'sub Debug () { 0 }'.
>
> From perlsub:
>
> # Functions with a prototype of "()" are potential candidates for
> # inlining. If the result after optimization and constant folding is
> # either a constant or a lexically‐scoped scalar which has no other
> # references, then it will be used in place of function calls made
> # without "&". Calls made using "&" are never inlined. (See
> # constant.pm for an easy way to declare most constants.)
>
>
> Eirik
sub DEBUG () { 0 } # Set to 0 for production; 1 for development
so it isn't getting optimized out, nonetheless.
Nicholas Clark
> Oops, forgot to put the function in.
> karl williamson wrote:
> >So, I tried it with NYTProf. As I expected (I had used DProf earlier),
> >the highest usage subroutine was my pure Perl version of
> >Scalar::Util::refaddr, reproduced below. A third of the total time was
> >spent in this routine. (This is required because miniperl doesn't do
> >dynamic loading, so refaddr is not available.)
> >
> >When I was writing mktables, I was under the impression that refaddr
> >would be brought into the core for 5.12. There was an agreement to that
> >effect, but I guess no one ever got around to actually doing it.
> >
> >When I run this under perl instead of miniperl, and change objaddr to
> >just return refaddr, the combination still takes quite a lot of time. If
> >refaddr were in the core would it be in-lined?
> sub objaddr($) {
> # Returns the address of the blessed input object. Uses the XS
> version if
> # available. It doesn't check for blessedness because that would do a
> # string eval every call, and the program is structured so that this is
> # never called for a non-blessed object.
>
> return Scalar::Util::refaddr($_[0]) if $has_fast_scalar_util;
>
> # Get the package
> my $pkg = ref($_[0]) or return undef;
>
> # Change to a fake package to defeat any overloading
> bless $_[0], 'main::Fake';
>
> # Numifying a ref gives its address.
> my $addr = 0 + $_[0];
>
> # Return to original class
> bless $_[0], $pkg;
> return $addr;
> }
>
> I found that any overload in a class caused the numifying to fail if I
> did it in that class; hence the blesses are necessary.
I have memory of comments being made on IRC about no overloading. Using it
avoids the need to rebless. With this patch:
diff --git a/lib/unicore/mktables b/lib/unicore/mktables
index ee51608..b2624f0 100644
--- a/lib/unicore/mktables
+++ b/lib/unicore/mktables
@@ -1133,17 +1133,11 @@ sub objaddr($) {
return Scalar::Util::refaddr($_[0]) if $has_fast_scalar_util;
# Check at least that is a ref.
- my $pkg = ref($_[0]) or return undef;
-
- # Change to a fake package to defeat any overloaded stringify
- bless $_[0], 'main::Fake';
+ ref($_[0]) or return undef;
# Numifying a ref gives its address.
- my $addr = 0 + $_[0];
-
- # Return to original class
- bless $_[0], $pkg;
- return $addr;
+ no overloading;
+ return 0 + $_[0];
}
sub max ($$) {
I find that on "my machine" the run time (with ./miniperl) goes down from 47 to
38 seconds. That's pretty close to the run time with ./perl (and hence
real Scalar::Util::refaddr) of 35.5 seconds.
Technically, I think, this falls foul of a strict reading of the feature
freeze, as it's not a bug fix. Unless "it's too slow" is considered a bug.
Nicholas Clark
Karl Williamson
Many thanks
>
> Technically, I think, this falls foul of a strict reading of the feature
> freeze, as it's not a bug fix. Unless "it's too slow" is considered a bug.
>
Ah, but it isn't a new feature; I could argue that mktables isn't even a
new feature, but no point; it met the deadline anyway. I'll submit a
patch with your fix. I'm trying to keep mktables compatible with 5.8 in
case a user wants to apply it; no object recompilations are needed. 'no
overloading' is not a 5.8 feature, so I'll add something to redefine it
when that pragma is not available.
> Nicholas Clark
>
Tim Bunce
>
> There are two columns in the nytprofhtml output for subroutines that I
> can't figure out what they mean, and saw no documentation for, 'P' and 'F'.
They're the number of 'places' the sub was called from (distinct file +
line number) and the number of distinct files. Hovering over the P and
the F should show a tooltip saying something along those lines.
Tim.
Eric Brine
<pub...@khwilliamson.com>wrote:
> I looked and actually it is that:
> sub DEBUG () { 0 } # Set to 0 for production; 1 for development
>
> so it isn't getting optimized out, nonetheless.
>
$ perl -Ilib -MO=Concise,-exec -e'sub DEBUG () { 1 } foo() if DEBUG'
1 <0> enter
2 <;> nextstate(main 2 -e:1) v:{
3 <0> pushmark s
4 <#> gv[*foo] s/EARLYCV
5 <1> entersub[t2] vKS/TARG,1
6 <@> leave[1 ref] vKP/REFC
-e syntax OK
$ perl -Ilib -MO=Concise,-exec -e'sub DEBUG () { 0 } foo() if DEBUG'
1 <0> enter
2 <;> nextstate(main 2 -e:1) v:{
3 <@> leave[1 ref] vKP/REFC
-e syntax OK
In both cases, the "if" is opimised away. And in the case where DEBUG
returns 0, so is the function call.
Jesse
On Thu, Nov 26, 2009 at 06:57:51AM +0100, Aristotle Pagaltzis wrote:
> * karl williamson <pub...@khwilliamson.com> [2009-11-26 06:35]:
> > As I expected (I had used DProf earlier), the highest usage
> > subroutine was my pure Perl version of Scalar::Util::refaddr,
> > reproduced below. A third of the total time was spent in this
> > routine. (This is required because miniperl doesn't do dynamic
> > loading, so refaddr is not available.)
>
> There was consensus for putting the Scalar::Utils stuff in core
> a while ago??? I wonder if that???s still on anyone???s radar.
There was, iirc, some debate over "the right way" to do it - and I never
saw a patch.
Demerphq
> Attached is a patch to cut down the frequency of mktables running. It turns
> out the problem was a combination of both Makefile and mktables. I didn't
> realize that the files I removed were listed as dependencies in Makefile, so
> it always thought it was out-of-date, and called mktables. Part of the old
> code I hadn't changed caused mktables to remove all the files when called
> with the -makelist option. This code is no longer necessary, so I removed
> it.
>
> I also changed Makefile to always call mktables, to let it decide if it
> actually should recompile. I just commented it out for easy reversal. The
> problem is that mktables depends on almost 800 files, located in the To and
> lib subdirectories, and Makefile doesn't know about them all; and they
> change from Unicode release to release. I'm assuming mktables.lst was
> created because of the issues of trying to get them all known by Makefile.
Yes, and also because we used to basically just generate them all, and
there was no list, we just had dependencies on a few files, and for
some irritating reason mktables would run like, umm, 6 times on win32.
And so back in the day it used to regenerate everything each of those
six times. So i taught it about the list of files and made it more
intelligent so that it doing things over and over turns into a noop on
following runs. Please dont make it too slow, in particular certain
stat calls on win32 are effectively a file open on every file in the
tree. If you do do that pleasee enable sloppy stat.
cheers,
Yves
Karl Williamson
mktables got it wrong about needing to run or not. On my Linux/Vista
box (this year's model) it compiles in under 30 seconds, and now doesn't
redo itself unless it should.
Demerphq
Win32 doesnt support the full linux stat interface. In order to
determine the nlink (iirc) count on a file on windows the file must be
opened and then the calls made to against the open handle. This means
that a File::Find on win32 is massively slower than a File::Find on
linux, and it is much much worse if the directory being searched is a
NTFS share.
The sloppy stat variable $^WIN32_SLOPPY_STAT (iirc) will disable the
nlink count and make an instant speedup on any directory or file
scanning code on windows that uses stat as it no longer needs to open
the file to calculate a value that is actually very rarely used in the
win32 world.
Anyway, thanks for this patch! Sounds great.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"