| <<<Back 1 day (to 2015/01/04) | 20150105 |
lolly | Hi everyone, I just want to make sure i am in the correct irc chat. I am trying to find out if anyone knows about Artifex Ghostscript and Primopdf - everytime i create a PDF a log file is created. Hope someone can help :) thanks so much | 07:01.43 |
jhabjan | svg device is discontinued, right? | 09:12.11 |
sebras | hi! | 12:11.47 |
| hi | 12:11.52 |
ghostbot | Welcome to #ghostscript, the channel for Ghostscript and MuPDF. If you have a question, please ask it, don't ask to ask it. Do be prepared to wait for a reply as devs will check the logs and reply when they come on line. | 12:11.52 |
sebras | ghostbot: thanks. | 12:11.56 |
ghostbot | no worries, sebras | 12:11.56 |
Robin_Watts | People can educate ghostbot by telling it things. You can then query it back. | 12:17.00 |
| ghostbot: tor8? | 12:17.06 |
ghostbot | tor8 is, like, a 6 and a half foot Viking with a distaste for C++. | 12:17.06 |
Robin_Watts | tor8: Did you see the mujs patches I emailed you over the break? | 12:21.28 |
tor8 | I've seen two patches from you | 12:22.22 |
| one which replaces 16 with sizeof | 12:22.31 |
| and one which adds const union funkiness | 12:22.38 |
Robin_Watts | Yeah, those two. | 12:22.43 |
tor8 | the 16 is intentional | 12:22.54 |
Robin_Watts | The first of those seems 'clean' and fixes segvs. | 12:23.01 |
| really? | 12:23.03 |
tor8 | struct { union { double d; char shrstr[8]; } char pad[7]; char tag; } | 12:23.15 |
| if you get segv's I'd like to know how... | 12:23.37 |
| sizeof that struct should be 16 | 12:23.49 |
Robin_Watts | http://ghostscript.com/regression/cgi-bin/clustermonitor.cgi?report=7209f8811b61007f999afcb78d4440a805d853f4&project=mujstest | 12:24.03 |
tor8 | with the 'tag' type at the end doubling as a string zero-terminator | 12:24.04 |
| argh, where did I misplace calc.pdf... | 12:27.04 |
| Robin_Watts: I can't reproduce those errors | 12:31.58 |
Robin_Watts | Let me try again. | 12:33.30 |
sebras | tor8: did you find it? | 12:35.33 |
tor8 | sebras: no, if you have it could you email it to me, so I don't lose it again? | 12:35.53 |
sebras | tor8: its in my account on casper. | 12:36.02 |
| tor8: I think this is the one. | 12:36.21 |
tor8 | sebras: thanks | 12:37.27 |
sebras | tor8: np. | 12:37.32 |
Robin_Watts | tor8: OK, I just did a mujstest cluster push with that sizeof fix in. | 13:19.39 |
| http://ghostscript.com/regression/cgi-bin/clustermonitor.cgi?report=robin | 13:19.44 |
| That looks pretty clean to me. | 13:19.56 |
| I'm going to try a clusterpush with the sizeof fix removed to double check. | 13:20.12 |
| Right, rerunning the test with the sizeof change removed causes all those SEGVs to appear. | 13:27.05 |
tor8 | Robin_Watts: strang. I can't get it to segv on my linux machine. | 13:27.26 |
| is the target 32-bit? | 13:27.29 |
Robin_Watts | I was on a 64bit VMware box when I ran it. | 13:27.44 |
| (and valgrind showed the problem) | 13:27.55 |
tor8 | I'm on a 64-bit linux and valgrind comes clean :( | 13:28.07 |
| maybe it's a gcc/clang issue | 13:28.26 |
Robin_Watts | tor8: What command line are you running? | 13:28.43 |
| (cd .. && valgrind --track-origins=yes mupdf.git/build/release/mujstest tests_private/pdf/forms/v1.2/key.mjs ) | 13:29.20 |
tor8 | valgrind ./build/debug/mujstest key.mjs | 13:29.27 |
Robin_Watts | tor8: I was testing the release build, but with -g added to the link stages so I could get sane valgrind results. | 13:30.10 |
tor8 | Robin_Watts: I'm testing the current mupdf origin/master | 13:31.35 |
| I can't get it to show any warnings at all... :/ | 13:31.43 |
| Linux 3.2.0-4-amd64, gcc (Debian 4.7.2-5) 4.7.2 | 13:32.44 |
Robin_Watts | debug shows no warnings for me (Ubuntu 14.10 64bit), rebuilding release now. | 13:33.24 |
| but the vanilla cluster shows it, so it's not specific to my machine. | 13:33.44 |
| Right. a vanilla release build shows it for me. | 13:35.33 |
| installing clang now. | 13:36.05 |
| A clang build runs clean | 13:41.05 |
| yes, just rebuild with gcc, and it shows the errors. | 13:48.01 |
| So either it's a gcc bug (seems unlikely to me), or there is an issue that clang is masking. | 13:48.44 |
| malc_: Hi. | 14:00.04 |
| Just looking at that last change of yours now. | 14:00.14 |
| I think it's wrong, but in a fixable way. | 14:00.24 |
| malc_: http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=e460246943441078ff28dd413a19ee6f186c3764 | 14:01.02 |
| That's the fixed version that I'm testing now. | 14:01.11 |
| tor8, sebras, paulgardiner: I think that commit is ready for review whenever one of you has a chance. | 14:03.50 |
| I'm going to trudge back to SOT now unless anyone can think of an excuse for me not to. | 14:04.14 |
paulgardiner | I can take a look. Should be in my area. | 14:05.04 |
malc_ | Robin_Watts: yeah it wass very wrong | 14:05.09 |
paulgardiner | If you have a moment, I have a little sot commit ready for review | 14:05.32 |
Robin_Watts | malc_: If you could test the new version and let me know a) if it works, and b) how the speed compares for you now, that would be really useful. | 14:05.49 |
| I have an idea how we can improve the speed further, but I only want to pursue it if we need to. | 14:06.09 |
| paulgardiner: I will look now. | 14:06.14 |
malc_ | Robin_Watts: give me a moment | 14:06.37 |
Robin_Watts | malc_: No hurry! | 14:06.48 |
| paulgardiner: on paul/master? | 14:07.11 |
paulgardiner | yep | 14:07.24 |
tor8 | Robin_Watts: which gcc version are you using? | 14:09.18 |
Robin_Watts | restarts vmware box... | 14:09.35 |
tor8 | or do you have a machine I can log onto to test? | 14:09.43 |
Robin_Watts | gcc (Ubuntu 4.9.1-16ubuntu6) 4.9.1 | 14:10.07 |
| tor8: peeves? or peeved? | 14:10.12 |
tor8 | peeves has gcc 4.6.3, but I can give it a try there | 14:10.49 |
malc_ | Robin_Watts: very modest 0.002 seconds with speed.c which i shared with you the other day | 14:10.54 |
| modest improvement that is | 14:11.06 |
tor8 | I mean peeved. I can't log on to peeves anymore. | 14:11.24 |
| probably misplaced the ssh key | 14:11.29 |
Robin_Watts | malc_: That's the improvement between ARMARMv8 on the original (pre sparse xref) and the latest code ? | 14:11.36 |
malc_ | Robin_Watts: that's ARMARMv8 HEAD vs HEAD + your patch | 14:11.59 |
Robin_Watts | malc_: Ah, so we're still significantly slower than the pre sparse xref code ? | 14:12.19 |
malc_ | Robin_Watts: yes | 14:12.25 |
Robin_Watts | OK. | 14:12.28 |
| So, time for a conflab... | 14:12.46 |
| tor8, paulgardiner: So for this particular file that malc has found, we're much slower than before (a factor of 5). | 14:13.19 |
tor8 | Robin_Watts: no valgrind errors on peeved :( | 14:13.35 |
Robin_Watts | The file in question has loads of xrefs, each a small update on the others. | 14:13.40 |
malc_ | Robin_Watts: fwiw running mutool clean on that yields nicely fast pdf (the result is many times fatter though) | 14:14.37 |
Robin_Watts | malc_: Yes, that's because we flatten to a single xref (hence much faster), but lose the object stream compression. | 14:15.10 |
paulgardiner | Robin_Watts: so it isn't that the individual xrefs have many sections, just that there are many xrefs? | 14:15.33 |
Robin_Watts | It's the fact that there is lots of searching. | 14:15.52 |
| I wouldn't like to say which of those two it was. | 14:16.04 |
| I had an idea of how to improve the problem. | 14:16.26 |
paulgardiner | It's just that we required searching of xrefs before your changes... I thought. | 14:16.32 |
malc_ | Robin_Watts: you don't really want to "improve the problem" :) | 14:16.44 |
paulgardiner | although not before my changes some time ago. | 14:16.52 |
Robin_Watts | paulgardiner: My change was to make it so that there were multiple things to search for each xref rather than a single one. | 14:17.38 |
| so it must be that that's the problem. | 14:17.44 |
| My cunning idea to get around this is to introduce a table that maps from object number to 'first xref where that object number appears'. | 14:18.18 |
| Hence we can cut out the vast majority of searching. | 14:18.53 |
| The only problem is when we move an object between xrefs, and we already have a function call for that particular case. | 14:19.21 |
paulgardiner | Is there a reason we couldn't keep the individual xrefs, plus a single consolidated one? | 14:19.36 |
Robin_Watts | It's just a matter of making that update an additional table entry. | 14:19.37 |
tor8 | what do we actually need the old xrefs for? | 14:19.51 |
| to load old versions of the file for digital signatures | 14:19.58 |
| or anything else? | 14:20.02 |
Robin_Watts | tor8: that, plus incremental updates, AIUI. | 14:20.11 |
paulgardiner | tor8: yep | 14:20.18 |
| ... not that we currently support what that would be needed for. | 14:20.23 |
tor8 | saving out an incremental update should be trivial with just a dirty bit in each xref entry | 14:20.30 |
| what if we do like paul said, hold a consolidated 'this is the real deal' xref for all normal use | 14:21.14 |
Robin_Watts | tor8: Depends if you want to be able to load file X, then make changes and save an incremental version Y, then make more changes and save out Y2 (which is incremental from X, not Y) | 14:21.22 |
tor8 | and then keep a linked list of the original xref entries for digital signatures? | 14:21.40 |
Robin_Watts | paulgardiner, tor8: I considered that (briefly) and was scared off by the fact that that was a change to the internal assumptions we'd been making all along. | 14:22.12 |
tor8 | I don't see the case for that | 14:22.13 |
| either we're saving incrementally from the last saved, or a full save with all new stuff? | 14:22.27 |
| or is this to do with signatures again? | 14:22.36 |
| before we added digital signatures, it was always one big consolidated xref | 14:23.08 |
Robin_Watts | Suppose I open file X in an editor, and I sign it. Then I save it out as Y.T | 14:23.23 |
tor8 | which is what caused the original problem wiht memory bloat, we were holding consolidated xrefs for each incremental version | 14:23.30 |
Robin_Watts | Then I realise that I signed it wrongly. I want to re-sign it and save it again. | 14:23.53 |
| I'd rather have something based on X, not something based on Y. | 14:24.06 |
paulgardiner | Just to throw this out there: since we searched xrefs before the recent change, I wonder if the slowness is because of searching the linked listed subsections, in which case we could maybe use an alternative to linked list to achieve sparseness | 14:24.07 |
Robin_Watts | paulgardiner: We could, but whatever we pick will be slower than the array. | 14:24.32 |
| The idea of an additional index (a cache if you like) has the advantages that: | 14:24.51 |
paulgardiner | Maybe log n rather than n slower though | 14:25.04 |
Robin_Watts | 1) it doesn't change our fundamental meanings of the existing data structures. | 14:25.09 |
| 2) it neatly solves this problem (I think) | 14:25.21 |
paulgardiner | Yeah the cache does have a sort of naturalness to it | 14:25.32 |
Robin_Watts | 3) it's less memory than having a consolidated xref has. | 14:25.41 |
| 4) it doesn't result in duplicated information | 14:26.00 |
paulgardiner | Oh I thought you were refering to the single consolidated xref as a cache | 14:26.29 |
Robin_Watts | 5) it doesn't require any API changes as it fits into the structure we already have. | 14:26.36 |
Robin_Watts | is called for lunch. bbs, sorry. | 14:26.47 |
paulgardiner | cyl | 14:27.19 |
tor8 | Robin_Watts: I'm stumped. I can't reproduce the SEGV's on *any* of my machines. I've tried debian, ubuntu, peeved. | 14:28.52 |
paulgardiner | Ahhhh! In any case, I was imagining we'd load the file into separate xrefs and then make the consoldited one. Since it's loading that is being slowed, I guess that wouldn't help. Doh! | 14:29.12 |
tor8 | gcc versions 4.8.1, 4.7.2, 4.6.3 | 14:29.29 |
Robin_Watts | ok, back. | 15:07.23 |
| tor8: Can you reproduce the valgrind errors? | 15:07.41 |
tor8 | Robin_Watts: nope. nothing. | 15:28.25 |
| anything I try, it works clean. | 15:28.30 |
Robin_Watts | tor8: Let me try and get you a login to my vmware box. | 15:28.58 |
paulgardiner | robin: commit is fine | 15:33.37 |
tor8 | Robin_Watts: if you add these lines to js_newstate in jsstate.c and run ./build/mujs what do you see printed? | 15:39.14 |
| printf("sizeof js_Value = %lu\n", sizeof(js_Value)); | 15:39.17 |
| printf("offsetof js_Value.type = %lu\n", offsetof(js_Value, type)); | 15:39.17 |
malc_ | tor8: those two statements are a bit broken for win64 aren't they? | 15:41.57 |
tor8 | malc_: possibly, but I only want him to run it on linux64 | 15:43.00 |
malc_ | okay | 15:43.11 |
tor8 | robin is seeing odd crashes somehow related to js_Value shortstrings | 15:43.31 |
Robin_Watts | trying now. | 15:43.32 |
tor8 | which I cannot reproduce, and the only thing that I can think of that would cause it is if those two values aren't 16 and 15 | 15:43.51 |
Robin_Watts | tor8: You need some more makefile magic to detect changes in mujs not in one.c | 15:44.11 |
malc_ | tor8: 15 is a very weird value for offset though | 15:44.38 |
tor8 | one.c depends on all the other c files | 15:44.44 |
Robin_Watts | tor8: Except it doesn't as far as make is concerned. | 15:45.08 |
tor8 | malc_: struct js_Value { union { double d; char shrstr[8]; } char pad[7]; char type; } | 15:45.12 |
Robin_Watts | tor8: 16 and 15. | 15:45.24 |
tor8 | Robin_Watts: one.c: $(SRCS) | 15:45.34 |
malc_ | tor8: ah.. type is char | 15:45.42 |
Robin_Watts | robin@osboxes:~/sauce/mupdf.git$ make build=releasemake: Nothing to be done for 'default'. | 15:46.07 |
tor8 | Robin_Watts: okay. I have *no* idea what is causing these cluster crashes that you can reproduce locally. | 15:46.09 |
| Robin_Watts: ah, the mupdf thirdparty makefiles don't trigger rebuilds on the submodules | 15:46.30 |
| there are lots of missing dependencies with the thirdparty makefiles :( | 15:48.36 |
Robin_Watts | tor8: OK, so the string in question is "Document" | 15:49.01 |
tor8 | is it erroring in js_pushstring, with "Document" as input? | 15:51.02 |
Robin_Watts | It is. | 15:51.38 |
| shrstr = 0x1545220 | 15:51.48 |
| pad = 0x1545228 | 15:51.50 |
| type = 0x154522f | 15:51.52 |
| v = 'Document' | 15:51.54 |
tor8 | n = 8 I take it? | 15:52.21 |
Robin_Watts | I assume so, haven't printed that :) | 15:52.32 |
tor8 | memcpy(shrstr, "Document", 8) should fill shrstr completely | 15:53.07 |
| then shrstr[n] = 0 should write to pad[0] | 15:53.20 |
| could it be gcc going bonkers with pointer aliasing and optimizations? | 15:54.06 |
| you mentioned using a newer gcc than I could find on any of my machines | 15:54.33 |
Robin_Watts | urgh. | 15:54.38 |
| If i change to just setting the bytes manually we have no problems. | 15:54.57 |
| It's only strcpy. | 15:55.10 |
tor8 | Robin_Watts: oh, right. I've got a further cleanup here that just does memcpy and manual zero-termination | 15:55.34 |
Robin_Watts | and it's a buffer overflow thing in strcpy that is dying. | 15:55.35 |
| I suspect memcpy would be bad too (if it overflows the end of the buffer) | 15:56.00 |
tor8 | Robin_Watts: does the same thing happen if you run 'mujs' on the command line? | 15:56.41 |
Robin_Watts | Possibly a better workaround here would be to have a struct { char [sizeof(js_Value)] buf; } | 15:56.49 |
tor8 | just run ./build/mujs and enter a='Document' | 15:56.53 |
Robin_Watts | and then cast to that. | 15:56.56 |
tor8 | Robin_Watts: could just cast the whole js_Value to a char* when doing shrstr creation | 15:57.33 |
| but it would be good to know if it's just gcc making a mess | 15:57.54 |
Robin_Watts | I suspect it's gcc correctly spotting that you are doing something nasty. | 15:58.10 |
| Can you suggest a good cast we could use? | 15:58.28 |
tor8 | I expect it's going nuts with strcpy builtin optimisations... | 15:58.55 |
| a manual strcpy loop would do the trick | 15:59.09 |
| gcc builtin strcpy caused a *lot* of valgrind warnings at one point, due to SSE optimizations that read out of bounds | 15:59.41 |
Robin_Watts | tor8: A cast to a char buffer of the correct size would be nicer, IMAO. | 15:59.44 |
tor8 | strcpy((char[sizeof js_Value)shrstr, v) ? | 16:00.05 |
| that might be enough to trick gcc | 16:00.26 |
Robin_Watts | tor8: Let me try. | 16:00.27 |
| Dislikes casting to an array type. I think the structure may be required. | 16:02.23 |
tor8 | char *s = shrstr; then strcpy to s? | 16:02.52 |
Robin_Watts | just tried that. also failed. | 16:03.05 |
tor8 | yeah. it's probably trying to be smart :( | 16:03.13 |
| and remembering that s is just an alias | 16:03.23 |
| does it still crash if you change to memcpy and manual zero-termination? | 16:04.03 |
| like in js_pushlstring | 16:04.20 |
Robin_Watts | Works with the struct. | 16:04.53 |
malc_ | Robin_Watts: what's the gcc version you've got there? | 16:07.16 |
Robin_Watts | malc_: 4.9.1 | 16:07.47 |
| tor8: memcpy seems to work. | 16:08.12 |
| So that's probably the best idea (might be worth a comment to say why), and really, using '16' is evil. sizeof(x) is the better way to go, right? | 16:08.44 |
malc_ | Robin_Watts: and a="Document" does trigger it? | 16:08.49 |
Robin_Watts | malc_: Nope. | 16:08.57 |
malc_ | Robin_Watts: what does? | 16:09.03 |
Robin_Watts | Using mujstest | 16:09.10 |
malc_ | Robin_Watts: don't have that one here | 16:09.58 |
tor8 | Robin_Watts: mujstest is probably built with different optimization flags than the mujs binary | 16:10.29 |
| Robin_Watts: on mujs tor/master there's a commit that rewrites the shrstr to just use plain loops | 16:11.03 |
| simplest workaround I can think of | 16:11.12 |
| could you test that to see if it also solves the issues? | 16:11.37 |
| Robin_Watts: ah, right. so I already had the change to use memcpy. | 16:12.07 |
| but gcc failing on strcpy makes me worry about using memcpy; who knows when that'll break too :( | 16:12.30 |
| n <= offsetof(js_Value, type) might be the safest actually | 16:12.55 |
Robin_Watts | tor8: that sounds nicer than a hardwired number, yes. | 16:13.17 |
| I don't altogether understand why it's done like this... | 16:13.35 |
tor8 | Robin_Watts: in order to pass around short strings in the js_Value struct | 16:13.48 |
| and allow them to be as long as can fit | 16:13.54 |
| removes the need to allocate garbage collected memory for short strings | 16:14.10 |
| and given how string-intensive javascript is... | 16:14.23 |
Robin_Watts | Oh, I see, it's because of struct alignments etc. | 16:14.42 |
tor8 | given that the js_Value struct already has to be 16 bytes long | 16:14.47 |
| tricking it to use the full 16 bytes by having the type tag be the last byte and 0 meaning short string, the last byte serves double purpose | 16:15.21 |
| and lets us have 15 byte long strings | 16:15.26 |
| as opposed to 7 bytes if we were to do it "cleanly" | 16:15.36 |
Robin_Watts | Yes, I see now. | 16:15.47 |
| That's worthy of a comment, I feel. | 16:16.00 |
| tor8: building now. | 16:20.12 |
| tor8: http://pastebin.com/Gi2uquHR | 16:20.45 |
tor8 | Robin_Watts: harmless warnings caused by gcc goofiness :( | 16:21.40 |
Robin_Watts | That runs clean, but personally, I prefer the memcpy one. | 16:21.46 |
| tor8: What is JS_TSHRSTR ? | 16:22.26 |
| 0 ? | 16:22.31 |
tor8 | yes. | 16:22.33 |
Robin_Watts | cunning. | 16:22.37 |
tor8 | JS_TSHRSTR, /* type tag doubles as string zero-terminator */ | 16:22.44 |
| in the header. my (cryptic) comment that should explain the whole shenanigans :) | 16:23.09 |
| Robin_Watts: my most cunning code ever... I generally frown on constructs like this | 16:23.36 |
| but the gains were just too bountiful to pass up the opportunity | 16:23.45 |
Robin_Watts | I see no meaningful comment in jsvalue.h near the js_Value struct. | 16:24.27 |
| I'd be tempted to do something like: | 16:24.51 |
| enum { js_Value_MaxShrStrLen = sizeof(js_Value)-1 }; | 16:25.27 |
| or the same thing using offsetof. | 16:26.08 |
tor8 | Robin_Watts: something like the top commit on mujs tor/master? | 16:32.24 |
Robin_Watts | Laverly. | 16:33.21 |
| Hi Fred. Happy New Year. | 17:03.54 |
sebras | tor8: I'll take the fixed mujs for a spin. | 17:16.34 |
Robin_Watts | mvrhel_laptop: Power back? | 19:58.51 |
mvrhel_laptop | Yes. And, equally important, internet! | 19:59.07 |
| power came back up at 7:30am but internet was a bit delayed | 20:00.37 |
malc_ | Robin_Watts: even one element cache gives a ~1.7 speed boost to pdf_get_xref_entry btw | 20:11.28 |
Robin_Watts | malc_: Right. | 20:11.48 |
| I'm working on the proper fix now (but being sidetracked) | 20:12.00 |
malc_ | Robin_Watts: nice | 20:12.57 |
henrys | hopefully the weather will be nicer for the next meeting no fun in a plane with these winds: http://www.denverpost.com/news/ci_27261298/colorado-wind-gust-reports-jan-5-2015?source=rss | 20:26.31 |
Robin_Watts | henrys: Most importantly, we're looking at doing a snowmobile thing in Vail. Weather better be good for that! | 20:44.03 |
henrys | you'll be fine, this is pretty rare. It's weird it was subzero this morning and everything was ice, now you can go outside without a jacket on. The natives called them chinooks - snow eaters. | 20:45.42 |
| the high winds that is. | 20:46.01 |
| hi fredross-perry - not sure if I asked you to the meeting tomorrow? Can you stop in? | 20:48.41 |
fredross-perry | yes I will. Been OOT. Catching up on the packaging dicussion now. | 20:49.11 |
| Still 7:30 PST? | 20:49.57 |
henrys | fredross-perry: yes | 20:52.08 |
fredross-perry | good deal. | 20:52.28 |
marcosw | Power is out at my house, so the ATS is offline. PGE estimates the power will be restored by 6:30pm PST. | 23:27.30 |
| Forward 1 day (to 2015/01/06)>>> | |