Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(651)

Issue 548030043: Add option to use Ghostscript API instead of forking (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 7 months ago by hahnjo
Modified:
1 year ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Add option to use Ghostscript API instead of forking This is much more efficient than the current scheme when converting many PostScript files, for example when building the documentation. For the Notation Reference, lilypond-book now takes ~2m30s instead of 5m to compile all snippets from the notation.tely file. Other manuals benefit less, but still the time for 'make doc' on my system improves by one third from around 33m to 22m. An alternative would have been to always call gsapi_init_with_args with the same arguments used until now. This indeed works and avoids the overhead of forking, but the instance cannot be reused. Calling gsapi_new_instance -> gsapi_init_with_args -> gsapi_delete_instance for every conversion still means a lot of overhead and a prototype suggested only a small gain compared to the previous solution. This is not the default because recent versions of Ghostscript are distributed under the AGPL and it's unclear what the implications of linking to the library is. If built with support for the API, use -dgs-api=#f to still fork the gs command.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Add option to use Ghostscript API instead of forking #

Total comments: 2

Patch Set 3 : add license information #

Total comments: 2

Patch Set 4 : rebase + allow -dgs-api=#f to still fork #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -27 lines) Patch
M config.hh.in View 1 1 chunk +3 lines, -0 lines 0 comments Download
M config.make.in View 1 1 chunk +1 line, -0 lines 0 comments Download
M configure.ac View 1 1 chunk +21 lines, -0 lines 0 comments Download
M lily/GNUmakefile View 1 1 chunk +3 lines, -0 lines 0 comments Download
M lily/general-scheme.cc View 1 2 3 2 chunks +129 lines, -0 lines 0 comments Download
M lily/main.cc View 1 2 3 3 chunks +30 lines, -0 lines 0 comments Download
M scm/backend-library.scm View 1 2 chunks +17 lines, -15 lines 0 comments Download
M scm/lily.scm View 1 2 3 2 chunks +5 lines, -0 lines 0 comments Download
M scm/ps-to-png.scm View 1 2 chunks +23 lines, -12 lines 1 comment Download

Messages

Total messages: 38
hahnjo
This probably goes without saying, but touching such core functionality should receive very rigid testing. ...
1 year, 7 months ago (2020-04-30 19:15:23 UTC) #1
dak
On 2020/04/30 19:15:23, hahnjo wrote: > This probably goes without saying, but touching such core ...
1 year, 7 months ago (2020-04-30 19:37:56 UTC) #2
lemzwerg
LGTM, thanks! Looks very good, indeed.
1 year, 7 months ago (2020-04-30 19:52:09 UTC) #3
hanwenn
Technologically speaking, this is a brilliant idea, and I am all in favor it. However, ...
1 year, 7 months ago (2020-05-01 06:28:56 UTC) #4
hanwenn
https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc File lily/general-scheme.cc (right): https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc#newcode783 lily/general-scheme.cc:783: command += "(" + ly_scm2string (input) + ") run"; ...
1 year, 7 months ago (2020-05-01 06:42:43 UTC) #5
hahnjo
disclaimer: I'm not a lawyer, this is just my understanding of the licenses. On 2020/05/01 ...
1 year, 7 months ago (2020-05-01 07:58:32 UTC) #6
hanwenn
On 2020/05/01 07:58:32, hahnjo wrote: > disclaimer: I'm not a lawyer, this is just my ...
1 year, 7 months ago (2020-05-01 09:49:35 UTC) #7
hahnjo
On 2020/05/01 09:49:35, hanwenn wrote: > LilyPond can also output postscript with embedded fonts, which ...
1 year, 7 months ago (2020-05-01 09:52:38 UTC) #8
dak
On 2020/05/01 07:58:32, hahnjo wrote: > disclaimer: I'm not a lawyer, this is just my ...
1 year, 7 months ago (2020-05-01 11:15:55 UTC) #9
hahnjo
https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc File lily/general-scheme.cc (right): https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc#newcode783 lily/general-scheme.cc:783: command += "(" + ly_scm2string (input) + ") run"; ...
1 year, 7 months ago (2020-05-01 11:18:11 UTC) #10
hanwenn
On Fri, May 1, 2020 at 1:18 PM <jonas.hahnfeld@gmail.com> wrote: > > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > ...
1 year, 7 months ago (2020-05-01 11:31:23 UTC) #11
hahnjo
On 2020/05/01 11:31:23, hanwenn wrote: > On Fri, May 1, 2020 at 1:18 PM <mailto:jonas.hahnfeld@gmail.com> ...
1 year, 7 months ago (2020-05-01 11:35:26 UTC) #12
dak
On 2020/05/01 11:18:11, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc#newcode783 > ...
1 year, 7 months ago (2020-05-01 11:41:13 UTC) #13
hahnjo
On 2020/05/01 11:41:13, dak wrote: > On 2020/05/01 11:18:11, hahnjo wrote: > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > ...
1 year, 7 months ago (2020-05-01 11:44:05 UTC) #14
dak
On 2020/05/01 11:44:05, hahnjo wrote: > On 2020/05/01 11:41:13, dak wrote: > > On 2020/05/01 ...
1 year, 7 months ago (2020-05-01 12:12:40 UTC) #15
Valentin Villenave
On 2020/05/01 12:12:40, dak wrote: > That being said, the situation regarding Scorio, a proprietary ...
1 year, 7 months ago (2020-05-01 21:58:47 UTC) #16
hanwenn
On Fri, May 1, 2020 at 11:58 PM <v.villenave@gmail.com> wrote: > > On 2020/05/01 12:12:40, ...
1 year, 7 months ago (2020-05-01 22:19:38 UTC) #17
dak
On 2020/05/01 22:19:38, hanwenn wrote: > On Fri, May 1, 2020 at 11:58 PM <mailto:v.villenave@gmail.com> ...
1 year, 7 months ago (2020-05-01 22:38:02 UTC) #18
hahnjo
Add option to use Ghostscript API instead of forking
1 year, 6 months ago (2020-05-02 09:45:01 UTC) #19
hahnjo
On 2020/05/01 06:28:56, hanwenn wrote: > I suggest we make this an option that you ...
1 year, 6 months ago (2020-05-02 09:49:44 UTC) #20
dak
On 2020/05/02 09:49:44, hahnjo wrote: > On 2020/05/01 06:28:56, hanwenn wrote: > > I suggest ...
1 year, 6 months ago (2020-05-02 10:04:14 UTC) #21
hahnjo
On 2020/05/02 10:04:14, dak wrote: > On 2020/05/02 09:49:44, hahnjo wrote: > > On 2020/05/01 ...
1 year, 6 months ago (2020-05-02 10:09:33 UTC) #22
hanwenn
On Sat, May 2, 2020 at 12:09 PM <jonas.hahnfeld@gmail.com> wrote: > > This version of ...
1 year, 6 months ago (2020-05-02 10:12:45 UTC) #23
hanwenn
https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc File lily/general-scheme.cc (right): https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc#newcode778 lily/general-scheme.cc:778: free (a); the code mixes setting up the GS ...
1 year, 6 months ago (2020-05-02 10:15:40 UTC) #24
hahnjo
On 2020/05/02 10:12:45, hanwenn wrote: > On Sat, May 2, 2020 at 12:09 PM <mailto:jonas.hahnfeld@gmail.com> ...
1 year, 6 months ago (2020-05-02 10:20:25 UTC) #25
hahnjo
https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc File lily/general-scheme.cc (right): https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc#newcode778 lily/general-scheme.cc:778: free (a); On 2020/05/02 10:15:40, hanwenn wrote: > the ...
1 year, 6 months ago (2020-05-02 10:22:15 UTC) #26
hahnjo
add license information
1 year, 6 months ago (2020-05-02 10:48:20 UTC) #27
hahnjo
On 2020/05/02 10:48:20, hahnjo wrote: > add license information $ lilypond -w GNU LilyPond 2.21.2 ...
1 year, 6 months ago (2020-05-02 10:49:11 UTC) #28
hanwenn
https://codereview.appspot.com/548030043/diff/548050047/lily/main.cc File lily/main.cc (right): https://codereview.appspot.com/548030043/diff/548050047/lily/main.cc#newcode345 lily/main.cc:345: printf ("%s", (_ ("linked against Ghostscript:").c_str ())); excellent, thanks ...
1 year, 6 months ago (2020-05-02 11:05:21 UTC) #29
hanwenn
LGTM https://codereview.appspot.com/548030043/diff/548050047/lily/general-scheme.cc File lily/general-scheme.cc (right): https://codereview.appspot.com/548030043/diff/548050047/lily/general-scheme.cc#newcode777 lily/general-scheme.cc:777: for (char *a : passed_args) I'll try to ...
1 year, 6 months ago (2020-05-02 11:07:12 UTC) #30
hahnjo
rebase + allow -dgs-api=#f to still fork
1 year, 6 months ago (2020-05-03 09:46:56 UTC) #31
dak
On 2020/05/03 09:46:56, hahnjo wrote: > rebase + allow -dgs-api=#f to still fork I decided ...
1 year, 6 months ago (2020-05-03 10:40:25 UTC) #32
dak
On 2020/05/02 10:22:15, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc#newcode778 > ...
1 year, 6 months ago (2020-05-03 10:44:24 UTC) #33
hahnjo
On 2020/05/03 10:40:25, dak wrote: > On 2020/05/03 09:46:56, hahnjo wrote: > > rebase + ...
1 year, 6 months ago (2020-05-03 10:46:49 UTC) #34
dak
On 2020/05/03 10:46:49, hahnjo wrote: > On 2020/05/03 10:40:25, dak wrote: > > On 2020/05/03 ...
1 year, 6 months ago (2020-05-03 11:38:56 UTC) #35
hahnjo
In case you're interested how I found out about this: https://www.hahnjo.de/blog/2020/05/06/recursively-timeing-processes.html before this change (commit ...
1 year, 6 months ago (2020-05-06 18:33:23 UTC) #36
Valentin Villenave
On 5/6/20, jonas.hahnfeld@gmail.com <jonas.hahnfeld@gmail.com> wrote: > In case you're interested how I found out about ...
1 year, 6 months ago (2020-05-06 22:53:49 UTC) #37
hanwenn
1 year, 6 months ago (2020-05-09 09:52:22 UTC) #38
https://codereview.appspot.com/548030043/diff/559960069/scm/ps-to-png.scm
File scm/ps-to-png.scm (right):

https://codereview.appspot.com/548030043/diff/559960069/scm/ps-to-png.scm#new...
scm/ps-to-png.scm:184: (if (not (= 1 anti-alias-factor))
please submit this as is, but for a follow-up, it would be interesting to get
rid of the homebuilt anti-aliasing, and use the GS DownScaleFactor variable
instead.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b