unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
@ 2015-06-24  1:35 Paul Eggert
  2015-06-24  7:06 ` Glenn Morris
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2015-06-24  1:35 UTC (permalink / raw)
  To: 20887; +Cc: Artur Malabarba

When I run 'make bootstrap' now it's veerrrrry slow, hanging after this output:

...
Reloading stale subr-x.el
Loading /home/eggert/src/gnu/emacs/tmn/lisp/emacs-lisp/subr-x.el (source)...
Reloading stale isearch.el
Loading /home/eggert/src/gnu/emacs/tmn/lisp/isearch.el (source)...

It pauses for a loooongg time here (minutes) and then finally continues.  It has 
to do this many times during 'make bootstrap', so 'make bootstrap' is now 
realllly slooooow.

This is no doubt due to commit c7a19e0c80ec6134ab6fb1950d3e1ac59a7b986f, 
recently installed in the master, so I'm CC'ing Artur Malabarba.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24  1:35 bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes Paul Eggert
@ 2015-06-24  7:06 ` Glenn Morris
  2015-06-24  7:52   ` Artur Malabarba
  2015-06-24 13:31   ` Paul Eggert
  0 siblings, 2 replies; 24+ messages in thread
From: Glenn Morris @ 2015-06-24  7:06 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 20887, Artur Malabarba

Paul Eggert wrote:

> When I run 'make bootstrap' now it's veerrrrry slow, hanging after this output:
>
> ...
> Reloading stale subr-x.el
> Loading /home/eggert/src/gnu/emacs/tmn/lisp/emacs-lisp/subr-x.el (source)...
> Reloading stale isearch.el
> Loading /home/eggert/src/gnu/emacs/tmn/lisp/isearch.el (source)...

How can a bootstrap have stale files...? Mine doesn't.
But yes, dumping bootstrap-emacs is now ridiculously slow (~ 5 minutes).
I would like to know where Artur bought his laptop [1], which by my
estimate must run at about 200GHz.

[1] http://lists.gnu.org/archive/html/emacs-devel/2015-06/msg00426.html





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24  7:06 ` Glenn Morris
@ 2015-06-24  7:52   ` Artur Malabarba
  2015-06-24 13:36     ` Paul Eggert
  2015-06-24 15:55     ` Glenn Morris
  2015-06-24 13:31   ` Paul Eggert
  1 sibling, 2 replies; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24  7:52 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 20887, Paul Eggert

I don't know. Frankly, a bootstrap of 5 min seems incredibly fast, :)
mine always takes at least 10.

When I reporte the change added 5 sec to compile time, I was testing
the following:
    (benchmark-run 1 (byte-compile-file (buffer-file-name)))
which reports 3.13 seconds for me.

Does anyone know what could cause these 4 seconds to become 5 min
during bootstrap?
Does bootstrap compile the file 60 times? =P

If botstrap loads that code without compiling first, then it could
take about 10 seconds, but that still doesn't account for so much.

2015-06-24 8:06 GMT+01:00 Glenn Morris <rgm@gnu.org>:
> Paul Eggert wrote:
>
>> When I run 'make bootstrap' now it's veerrrrry slow, hanging after this output:
>>
>> ...
>> Reloading stale subr-x.el
>> Loading /home/eggert/src/gnu/emacs/tmn/lisp/emacs-lisp/subr-x.el (source)...
>> Reloading stale isearch.el
>> Loading /home/eggert/src/gnu/emacs/tmn/lisp/isearch.el (source)...
>
> How can a bootstrap have stale files...? Mine doesn't.
> But yes, dumping bootstrap-emacs is now ridiculously slow (~ 5 minutes).
> I would like to know where Artur bought his laptop [1], which by my
> estimate must run at about 200GHz.
>
> [1] http://lists.gnu.org/archive/html/emacs-devel/2015-06/msg00426.html





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24  7:06 ` Glenn Morris
  2015-06-24  7:52   ` Artur Malabarba
@ 2015-06-24 13:31   ` Paul Eggert
  2015-06-24 15:58     ` Glenn Morris
  1 sibling, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2015-06-24 13:31 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 20887, Artur Malabarba

Glenn Morris wrote:

> But yes, dumping bootstrap-emacs is now ridiculously slow (~ 5 minutes).

That's just the beginning.  The real slowness comes later in the build.  Each 
time it's compiling something else, Emacs has to reload isearch.el (as 
isearch.elc doesn't exist yet), and this takes many minutes.  And it does this 
over and over.  The entire build takes so long that I haven't had the patience 
to let it finish.  Hours, at least.

> How can a bootstrap have stale files...? Mine doesn't.

I assume messages like "Reloading stale subr.el" are emitted even when the 
problem is only that subr.elc does not exist.  At least, that's what happens 
when I do this:

rm src/bootstrap-emacs src/emacs $(find . -name '*.elc')
make

The build creates 'src/bootstrap-emacs' and then goes on as follows.  Don't you 
see similar output?

   ...
   Dumping under the name emacs
   45219 pure bytes used
   : paxctl -zex emacs
   mv -f emacs bootstrap-emacs
   make -C ../lisp compile-first EMACS="../src/bootstrap-emacs"
   make[2]: Entering directory '/home/eggert/src/gnu/emacs/tmn/lisp'
     ELC      emacs-lisp/macroexp.elc
     ELC      emacs-lisp/cconv.elc
     ELC      emacs-lisp/byte-opt.elc
     ELC      emacs-lisp/bytecomp.elc
     ELC      emacs-lisp/autoload.elc
   make[2]: Leaving directory '/home/eggert/src/gnu/emacs/tmn/lisp'
   make[2]: Entering directory '/home/eggert/src/gnu/emacs/tmn/lisp'
     ELC      ../lisp/abbrev.elc
   Reloading stale byte-run.el
   Loading /home/eggert/src/gnu/emacs/tmn/lisp/emacs-lisp/byte-run.el (source)...
   Reloading stale backquote.el
   Loading /home/eggert/src/gnu/emacs/tmn/lisp/emacs-lisp/backquote.el (source)...
   ...

There are lots more "stale" messages like that.






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24  7:52   ` Artur Malabarba
@ 2015-06-24 13:36     ` Paul Eggert
  2015-06-24 14:28       ` Artur Malabarba
  2015-06-24 15:55     ` Glenn Morris
  1 sibling, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2015-06-24 13:36 UTC (permalink / raw)
  To: bruce.connor.am, Glenn Morris; +Cc: 20887

Artur Malabarba wrote:
> Does anyone know what could cause these 4 seconds to become 5 min
> during bootstrap?

I'm not sure, but during bootstrap, the compiler hasn't been compiled yet; it's 
being interpreted.  So it's very slow.  The build process attempts to compile 
the compiler first, so that the rest of the build finishes faster, but the early 
part of the bootstrap must be done with a slow compiler.

> Does bootstrap compile the file 60 times? =P

Yes, it seems so.  With a slow compiler.  It takes hours.  It's really bad.

Please try the recipe I gave Glenn:

rm -f src/bootstrap-emacs src/emacs $(find . -name '*.elc')
make






^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 13:36     ` Paul Eggert
@ 2015-06-24 14:28       ` Artur Malabarba
  2015-06-24 15:05         ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 14:28 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 20887

Yes, I see why that would cause a problem.
Does anyone have any idea how we could approach this, then?

That slow piece code has no need to be compiled so early, but I don't
know how to avoid that. I can move it to another file, but then
isearch will need to require that file anyway so the delay will still
be there.

Options I see
1. I could move the code to a separate file, and then only require
that file as part of the isearch command, so it wouldn't be loaded at
compile time.
2. I could just save that char-table directly in the file. It would be
large, but it should speed up the loading by a lot.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 14:28       ` Artur Malabarba
@ 2015-06-24 15:05         ` Eli Zaretskii
  2015-06-24 15:19           ` Eli Zaretskii
  2015-06-24 17:15           ` Glenn Morris
  0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-24 15:05 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887, eggert

> Date: Wed, 24 Jun 2015 15:28:02 +0100
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: 20887@debbugs.gnu.org
> 
> Yes, I see why that would cause a problem.
> Does anyone have any idea how we could approach this, then?
> 
> That slow piece code has no need to be compiled so early, but I don't
> know how to avoid that. I can move it to another file, but then
> isearch will need to require that file anyway so the delay will still
> be there.
> 
> Options I see
> 1. I could move the code to a separate file, and then only require
> that file as part of the isearch command, so it wouldn't be loaded at
> compile time.
> 2. I could just save that char-table directly in the file. It would be
> large, but it should speed up the loading by a lot.

I think 1 is better.  Make that variable autoloaded in isearch.el, and
the problem should be solved.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 15:05         ` Eli Zaretskii
@ 2015-06-24 15:19           ` Eli Zaretskii
  2015-06-24 17:18             ` Artur Malabarba
  2015-06-24 17:15           ` Glenn Morris
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-24 15:19 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887, eggert

> Date: Wed, 24 Jun 2015 18:05:38 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 20887@debbugs.gnu.org, eggert@cs.ucla.edu
> 
> > Date: Wed, 24 Jun 2015 15:28:02 +0100
> > From: Artur Malabarba <bruce.connor.am@gmail.com>
> > Cc: 20887@debbugs.gnu.org
> > 
> > Yes, I see why that would cause a problem.
> > Does anyone have any idea how we could approach this, then?
> > 
> > That slow piece code has no need to be compiled so early, but I don't
> > know how to avoid that. I can move it to another file, but then
> > isearch will need to require that file anyway so the delay will still
> > be there.
> > 
> > Options I see
> > 1. I could move the code to a separate file, and then only require
> > that file as part of the isearch command, so it wouldn't be loaded at
> > compile time.
> > 2. I could just save that char-table directly in the file. It would be
> > large, but it should speed up the loading by a lot.
> 
> I think 1 is better.  Make that variable autoloaded in isearch.el, and
> the problem should be solved.

Btw, did you try replacing a simple iteration through all the
characters with map-char-table?





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24  7:52   ` Artur Malabarba
  2015-06-24 13:36     ` Paul Eggert
@ 2015-06-24 15:55     ` Glenn Morris
  1 sibling, 0 replies; 24+ messages in thread
From: Glenn Morris @ 2015-06-24 15:55 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887, Paul Eggert

Artur Malabarba wrote:

> I don't know. Frankly, a bootstrap of 5 min seems incredibly fast, :)
> mine always takes at least 10.

A single step taking 5 minutes. Ie, 5 mins sitting there just loading
isearch.el to produce the bootstrap-emacs executable.

> When I reporte the change added 5 sec to compile time, I was testing
> the following:
>     (benchmark-run 1 (byte-compile-file (buffer-file-name)))
> which reports 3.13 seconds for me.

That's not a useful test of what happens during bootstrap.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 13:31   ` Paul Eggert
@ 2015-06-24 15:58     ` Glenn Morris
  2015-06-25  2:48       ` Paul Eggert
  0 siblings, 1 reply; 24+ messages in thread
From: Glenn Morris @ 2015-06-24 15:58 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 20887, Artur Malabarba

Paul Eggert wrote:

> That's just the beginning.  The real slowness comes later in the
> build.  Each time it's compiling something else, Emacs has to reload
> isearch.el (as isearch.elc doesn't exist yet), and this takes many
> minutes.  And it does this over and over. 

That means you aren't doing a clean build.

>> How can a bootstrap have stale files...? Mine doesn't.
>
> I assume messages like "Reloading stale subr.el" are emitted even when
> the problem is only that subr.elc does not exist.

No, they mean that subr.el is newer than your Emacs executable.

> rm src/bootstrap-emacs src/emacs $(find . -name '*.elc')
> make

> The build creates 'src/bootstrap-emacs' and then goes on as follows.
> Don't you see similar output?

No, because I do a proper bootstrap. Accept no imitations! :)
There are no "stale" messages in such a build, and the isearch slowness
only happens once.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 15:05         ` Eli Zaretskii
  2015-06-24 15:19           ` Eli Zaretskii
@ 2015-06-24 17:15           ` Glenn Morris
  2015-06-24 17:38             ` Artur Malabarba
  1 sibling, 1 reply; 24+ messages in thread
From: Glenn Morris @ 2015-06-24 17:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887, eggert, bruce.connor.am

Eli Zaretskii wrote:

>> Options I see
>> 1. I could move the code to a separate file, and then only require
>> that file as part of the isearch command, so it wouldn't be loaded at
>> compile time.
>> 2. I could just save that char-table directly in the file. It would be
>> large, but it should speed up the loading by a lot.
>
> I think 1 is better.  Make that variable autoloaded in isearch.el, and
> the problem should be solved.

(I didn't look at the details of this case, but autoloading variables
tends to just dump the entire definition in the target file, so that
might not help.)

Do note that

time ./src/emacs -Q -batch -l ./lisp/isearch.el

takes ~ 90 seconds here (versus ~ 0.1 sec for the .elc version).

So it's still leaving a time-bomb around if anyone ever loads the
uncompiled version of wherever that code ends up.

(Why is it so dog slow?)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 15:19           ` Eli Zaretskii
@ 2015-06-24 17:18             ` Artur Malabarba
  2015-06-24 19:14               ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 17:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

> Btw, did you try replacing a simple iteration through all the
> characters with map-char-table?

Hadn't thought of that. Indeed, the second loop can be replaced by a
map-char-table, and it speeds things up by about 30%.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 17:15           ` Glenn Morris
@ 2015-06-24 17:38             ` Artur Malabarba
  0 siblings, 0 replies; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 17:38 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Paul Eggert, 20887

2015-06-24 18:15 GMT+01:00 Glenn Morris <rgm@gnu.org>:
> Eli Zaretskii wrote:
>
>>> Options I see
>>> 1. I could move the code to a separate file, and then only require
>>> that file as part of the isearch command, so it wouldn't be loaded at
>>> compile time.
>>> 2. I could just save that char-table directly in the file. It would be
>>> large, but it should speed up the loading by a lot.
>>
>> I think 1 is better.  Make that variable autoloaded in isearch.el, and
>> the problem should be solved.
>
> (I didn't look at the details of this case, but autoloading variables
> tends to just dump the entire definition in the target file, so that
> might not help.)

I'll actually autoload the function that does the conversion.

> Do note that
>
> time ./src/emacs -Q -batch -l ./lisp/isearch.el
>
> takes ~ 90 seconds here (versus ~ 0.1 sec for the .elc version).
>
> So it's still leaving a time-bomb around if anyone ever loads the
> uncompiled version of wherever that code ends up.

I don't know. If I run that command I get similar results to you (~ 50
sec). But now I've moved the code to its own file, and running that
command on the new file yields 3 seconds (which is roughly the same I
get from evaluating it inside Emacs).
So maybe it's not such a big time bomb.

I'll push the change and we'll see if it helps.

> (Why is it so dog slow?)

No idea! But it's something related to isearch.el.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 17:18             ` Artur Malabarba
@ 2015-06-24 19:14               ` Eli Zaretskii
  2015-06-24 20:05                 ` Artur Malabarba
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-24 19:14 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887

> Date: Wed, 24 Jun 2015 18:18:51 +0100
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: 20887@debbugs.gnu.org
> 
> > Btw, did you try replacing a simple iteration through all the
> > characters with map-char-table?
> 
> Hadn't thought of that. Indeed, the second loop can be replaced by a
> map-char-table, and it speeds things up by about 30%.

Why only the second?  All you care about is decomposition, that is,
you need only loop over characters that have a non-nil value in the
decomposition property.  You will see in characters.el how you can use
map-char-table over a char-table loaded from uni-decomposition.el
(similar to what we do there with uni-bidi et.).  Won't that be much
faster?

(I didn't actually try that, so perhaps I'm talking nonsense.)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 19:14               ` Eli Zaretskii
@ 2015-06-24 20:05                 ` Artur Malabarba
  2015-06-24 23:17                   ` Artur Malabarba
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 20:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

> Why only the second?  All you care about is decomposition, that is,
> you need only loop over characters that have a non-nil value in the
> decomposition property.  You will see in characters.el how you can use
> map-char-table over a char-table loaded from uni-decomposition.el
> (similar to what we do there with uni-bidi et.).  Won't that be much
> faster?
>
> (I didn't actually try that, so perhaps I'm talking nonsense.)

You're perfectly correct. I wasn't aware there's a char table for the
decomposition property (or any of the char properties for that
matter). Thanks, I'll add it in.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 20:05                 ` Artur Malabarba
@ 2015-06-24 23:17                   ` Artur Malabarba
  2015-06-24 23:37                     ` Artur Malabarba
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 23:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

2015-06-24 21:05 GMT+01:00 Artur Malabarba <bruce.connor.am@gmail.com>:
>> Why only the second?  All you care about is decomposition, that is,
>> you need only loop over characters that have a non-nil value in the
>> decomposition property.  You will see in characters.el how you can use
>> map-char-table over a char-table loaded from uni-decomposition.el
>> (similar to what we do there with uni-bidi et.).  Won't that be much
>> faster?
>>
>> (I didn't actually try that, so perhaps I'm talking nonsense.)
>
> You're perfectly correct. I wasn't aware there's a char table for the
> decomposition property (or any of the char properties for that
> matter). Thanks, I'll add it in.

Actually, I spoke (and pushed) too soon. I have no idea how this works
The following code

(map-char-table
       (lambda (idx dec) (message "%S %S" idx dec))
       (unicode-property-table-internal 'decomposition))

calls the function a measly total of 100 times. The idx values are
things like below (which I understand), but the dec values are HUGE
strings (too big to post here, most of their content is the ^@ char)
which make no sense to me. Am I using this wrong?
(128 . 255)
(256 . 383)
(384 . 511)
(512 . 639)
(640 . 767)
(768 . 895)
(896 . 1023)
(1024 . 1151)
(1152 . 1279)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 23:17                   ` Artur Malabarba
@ 2015-06-24 23:37                     ` Artur Malabarba
  2015-06-25 15:03                       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-24 23:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

I think I see now. Reading the code for `get-char-code-property' shows
that I had to call a function stored in the extra slot.
Calling that function on the table seems to be sufficient to populate it.

2015-06-25 0:17 GMT+01:00 Artur Malabarba <bruce.connor.am@gmail.com>:
> 2015-06-24 21:05 GMT+01:00 Artur Malabarba <bruce.connor.am@gmail.com>:
>>> Why only the second?  All you care about is decomposition, that is,
>>> you need only loop over characters that have a non-nil value in the
>>> decomposition property.  You will see in characters.el how you can use
>>> map-char-table over a char-table loaded from uni-decomposition.el
>>> (similar to what we do there with uni-bidi et.).  Won't that be much
>>> faster?
>>>
>>> (I didn't actually try that, so perhaps I'm talking nonsense.)
>>
>> You're perfectly correct. I wasn't aware there's a char table for the
>> decomposition property (or any of the char properties for that
>> matter). Thanks, I'll add it in.
>
> Actually, I spoke (and pushed) too soon. I have no idea how this works
> The following code
>
> (map-char-table
>        (lambda (idx dec) (message "%S %S" idx dec))
>        (unicode-property-table-internal 'decomposition))
>
> calls the function a measly total of 100 times. The idx values are
> things like below (which I understand), but the dec values are HUGE
> strings (too big to post here, most of their content is the ^@ char)
> which make no sense to me. Am I using this wrong?
> (128 . 255)
> (256 . 383)
> (384 . 511)
> (512 . 639)
> (640 . 767)
> (768 . 895)
> (896 . 1023)
> (1024 . 1151)
> (1152 . 1279)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 15:58     ` Glenn Morris
@ 2015-06-25  2:48       ` Paul Eggert
  2015-06-25 23:21         ` Glenn Morris
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2015-06-25  2:48 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 20887-done, Artur Malabarba

Glenn Morris wrote:
> No, because I do a proper bootstrap. Accept no imitations!:)
> There are no "stale" messages in such a build, and the isearch slowness
> only happens once.

OK.  Still, running this from a fresh checkout:

./autogen.sh
./configure
make

also exhibited the "stale" messages and the extreme slowdown.  So the problem 
wasn't limited to my rm hack (which I do, by the way, because it's ordinarily 
much faster than "make bootstrap").

Anyway, the performance bug seems to be fixed now -- thanks! -- so I'm marking 
this as done.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-24 23:37                     ` Artur Malabarba
@ 2015-06-25 15:03                       ` Eli Zaretskii
  2015-06-25 17:32                         ` Artur Malabarba
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-25 15:03 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887

> Date: Thu, 25 Jun 2015 00:37:51 +0100
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: 20887@debbugs.gnu.org
> 
> I think I see now. Reading the code for `get-char-code-property' shows
> that I had to call a function stored in the extra slot.
> Calling that function on the table seems to be sufficient to populate it.

Indeed, the Unicode tables are tricky, in that with some of them you
need to call the function stored in the 1st extra slot to reconstruct
the value for a character.  (And if there's no function in that extra
slot, you should use get-unicode-property-internal to decode the
value.)

However, I think the version you pushed can be further improved.  For
starters, you don't need to populate the table first, and only then
use it; you can produce the property value for a character and use it
in the same call to map-char-table.  For example, the following
snippet loops only once over the table, and does its job in about 2
sec:

  (let* ((table (unicode-property-table-internal 'decomposition))
	 (func (char-table-extra-slot table 1)))
    (map-char-table
     (lambda (key val)
       (if (consp key)
	   (message "%s %s" key (funcall func (car key) val table))
	 (message "%s %s" key (funcall func key val table))))
     table))

All you need is replace the silly 'message' calls with the body of
your processing code (and handle all the characters in a range of
codepoints, not just the first one).

Your code also calls unicode-property-table-internal twice, and the
above method will solve that as well.

Thanks.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-25 15:03                       ` Eli Zaretskii
@ 2015-06-25 17:32                         ` Artur Malabarba
  2015-06-26  7:52                           ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-25 17:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

> However, I think the version you pushed can be further improved.  For
> starters, you don't need to populate the table first, and only then
> use it; you can produce the property value for a character and use it
> in the same call to map-char-table.  For example, the following
> snippet loops only once over the table, and does its job in about 2
> sec:
>
>   (let* ((table (unicode-property-table-internal 'decomposition))
>          (func (char-table-extra-slot table 1)))
>     (map-char-table
>      (lambda (key val)
>        (if (consp key)
>            (message "%s %s" key (funcall func (car key) val table))
>          (message "%s %s" key (funcall func key val table))))
>      table))
>
> All you need is replace the silly 'message' calls with the body of
> your processing code (and handle all the characters in a range of
> codepoints, not just the first one).

The "handle all characters in a range" part is the issue.

For me, the first time that snippet is run it calls the lambda a total
of 101 times (and barely takes an instant).
More importantly, every single time the key is a range, and the value
returned by the `funcall' is actually the decomposition of the *first*
char in that range (not the full range).
For instance, one of the messages I get from your snippet is:
    (64256 . 64383) (compat 102 102)

But if you call (get-char-code-property 64257 'decomposition) (note
the 7) you'll get a decomposition of (compat 102 105).

Maybe I'm missing some simplification here.
I'm aware I could run a loop inside the lambda going from (car key) to
(cdr key), and then do the `(funcall func ...)' on each item in the
range, but I fail to see how that would be faster than running a
second map-char-table. In fact, the number of steps involved is much
larger if you loop:
 - the current version has a map of 100 steps and another of 5721,
 - your proposal would have a map of 100 steps, each containing a loop
of about 120 steps, which totals to over 12000 steps (I counted).

Again, maybe I'm missing something, or maybe looping would be much
faster than that second `map-char-table', but it seems to me like it
would just be doing more work.

> Your code also calls unicode-property-table-internal twice, and the
> above method will solve that as well.

Yes, that was an oversight. My intention was to reuse the `table' var.
I'll fix that.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-25  2:48       ` Paul Eggert
@ 2015-06-25 23:21         ` Glenn Morris
  0 siblings, 0 replies; 24+ messages in thread
From: Glenn Morris @ 2015-06-25 23:21 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 20887

Paul Eggert wrote:

> OK.  Still, running this from a fresh checkout:
>
> ./autogen.sh
> ./configure
> make
>
> also exhibited the "stale" messages

I can't reproduce that, and I don't see how it could happen
(look at byte-compile-refresh-preloaded for the source of the stale messages).
Either there is another bug somewhere, or maybe somehow you aren't doing
what you think you are doing...





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-25 17:32                         ` Artur Malabarba
@ 2015-06-26  7:52                           ` Eli Zaretskii
  2015-06-26 11:41                             ` Artur Malabarba
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-26  7:52 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887

> Date: Thu, 25 Jun 2015 18:32:51 +0100
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: 20887@debbugs.gnu.org
> 
> I'm aware I could run a loop inside the lambda going from (car key) to
> (cdr key), and then do the `(funcall func ...)' on each item in the
> range, but I fail to see how that would be faster than running a
> second map-char-table. In fact, the number of steps involved is much
> larger if you loop:
>  - the current version has a map of 100 steps and another of 5721,
>  - your proposal would have a map of 100 steps, each containing a loop
> of about 120 steps, which totals to over 12000 steps (I counted).
> 
> Again, maybe I'm missing something, or maybe looping would be much
> faster than that second `map-char-table', but it seems to me like it
> would just be doing more work.

It's not faster, but it's not slower, either.  Looping is not what
takes time here, and if you think map-char-table can somehow magically
avoid any looping, you should look at its implementation.

What takes time here is map-char-table itself and the body of the
loop, where the data is processed.

For the record, I show the variant I had in mind below.  I timed it on
my system, and found it delivering the same speed as your version, up
to the system clock accuracy.

(defconst character-fold-table
  (eval-when-compile
    (let* ((equiv (make-char-table 'character-fold-table))
           (table (unicode-property-table-internal 'decomposition))
           (func (char-table-extra-slot table 1)))
      ;; Compile a list of all complex characters that each simple
      ;; character should match.
      (map-char-table
       (lambda (i dec)
         (when (consp i))
	   (let ((ifirst (car i))
		 (ilast (cdr i)))
	     ;; Ensure the table is populated for this range.
	     (funcall func ifirst dec table)
	     ;; Loop over all non-trivial decompositions in the range.
	     (while (<= ifirst ilast)
	       (setq dec (funcall func ifirst (aref table ifirst) table))
	       (or (not (consp dec))
		   (and (eq ifirst (car dec))
			(null (cdr dec)))
		   (progn
		     ;; Discard a possible formatting tag.
		     (when (symbolp (car dec))
		       (setq dec (cdr dec)))
		     (let ((d dec) k found multiletter)
		       (while (and d (not found))
			 (setq k (pop d))
			 ;; Is k a number or letter, per unicode standard?
			 (setq found
			       (memq (get-char-code-property k 'general-category)
				     '(Lu Ll Lt Lm Lo Nd Nl No))))
		       (if found
			   ;; Check if the decomposition has more than
			   ;; one letter, because then we don't want
			   ;; the first letter to match the
			   ;; decomposition.
			   (dolist (k d)
			     (when (memq
				    (get-char-code-property k 'general-category)
				    '(Lu Ll Lt Lm Lo Nd Nl No))
			       (setq multiletter t)))
			 ;; If there's no number or letter on the
			 ;; decomposition, take the first character in it.
			 (setq found (car-safe dec)))
		       ;; Add i to the list of characters that k can
		       ;; represent. Also possibly add its
		       ;; decomposition, so we can match multi-char
		       ;; representations like (format "a%c" 769)
		       (when (and found (not (eq ifirst k)))
			 (let ((chars (cons (char-to-string ifirst)
					    (aref equiv k))))
			   (aset equiv k
				 (if multiletter chars
				   (cons (apply #'string dec) chars))))))))
	       (setq ifirst (1+ ifirst)))))
	     table)

      ;; Convert the lists of characters we compiled into regexps.
      (map-char-table
       (lambda (i v) (let ((re (regexp-opt (cons (char-to-string i) v))))
                  (if (consp i)
                      (set-char-table-range equiv i re)
                    (aset equiv i re))))
       equiv)
      equiv))
  "Used for folding characters of the same group during search.")





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-26  7:52                           ` Eli Zaretskii
@ 2015-06-26 11:41                             ` Artur Malabarba
  2015-06-26 13:32                               ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Artur Malabarba @ 2015-06-26 11:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 20887

> It's not faster, but it's not slower, either.  Looping is not what
> takes time here, and if you think map-char-table can somehow magically
> avoid any looping, you should look at its implementation.

Not magic, witchcraft of course. Completely different things!

But seriously. My logic was just: there are 7k entries with a nil
value, `map-char-table' does the "funcall func + check for nil" stuff
in c code, whereas the loop does it in lisp code; so *if there's a
difference*, it's probably in favor of map-char-table.
But your test has shown that there's probably no significant difference.

That said, I was merely speculating. If you have any reason whatsoever
to prefer the loop (maybe it's even more robust if the table ever
changes) feel free to push, I'm not particular. :-)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes
  2015-06-26 11:41                             ` Artur Malabarba
@ 2015-06-26 13:32                               ` Eli Zaretskii
  0 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2015-06-26 13:32 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: 20887

> Date: Fri, 26 Jun 2015 12:41:52 +0100
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: 20887@debbugs.gnu.org
> 
> If you have any reason whatsoever to prefer the loop (maybe it's
> even more robust if the table ever changes) feel free to push, I'm
> not particular. :-)

I see no reasons to replace your code.





^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-06-26 13:32 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-24  1:35 bug#20887: 'make bootstrap' now verrrry slow due to recent isearch changes Paul Eggert
2015-06-24  7:06 ` Glenn Morris
2015-06-24  7:52   ` Artur Malabarba
2015-06-24 13:36     ` Paul Eggert
2015-06-24 14:28       ` Artur Malabarba
2015-06-24 15:05         ` Eli Zaretskii
2015-06-24 15:19           ` Eli Zaretskii
2015-06-24 17:18             ` Artur Malabarba
2015-06-24 19:14               ` Eli Zaretskii
2015-06-24 20:05                 ` Artur Malabarba
2015-06-24 23:17                   ` Artur Malabarba
2015-06-24 23:37                     ` Artur Malabarba
2015-06-25 15:03                       ` Eli Zaretskii
2015-06-25 17:32                         ` Artur Malabarba
2015-06-26  7:52                           ` Eli Zaretskii
2015-06-26 11:41                             ` Artur Malabarba
2015-06-26 13:32                               ` Eli Zaretskii
2015-06-24 17:15           ` Glenn Morris
2015-06-24 17:38             ` Artur Malabarba
2015-06-24 15:55     ` Glenn Morris
2015-06-24 13:31   ` Paul Eggert
2015-06-24 15:58     ` Glenn Morris
2015-06-25  2:48       ` Paul Eggert
2015-06-25 23:21         ` Glenn Morris

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).