| Time |
Nick |
Message |
| 00:03 |
jeff |
some relevant discussion: https://www.nntp.perl.org/group/perl.perl5.porters/2017/07/msg245703.html |
| 00:17 |
jeff |
"A downside of &PL_sv_zero is that if assigned to a variable, that variable gets int, num and string values rather than just an int value." |
| 00:33 |
|
dbwells joined #evergreen |
| 00:46 |
|
dbwells_ joined #evergreen |
| 03:05 |
|
dbwells joined #evergreen |
| 05:34 |
|
dbwells joined #evergreen |
| 06:00 |
pinesol |
News from qatests: Testing Success <http://testing.evergreen-ils.org/~live> |
| 07:25 |
|
rjackson_isl_hom joined #evergreen |
| 07:41 |
|
Dyrcona joined #evergreen |
| 07:42 |
Dyrcona |
typos-- |
| 07:45 |
Dyrcona |
Dropping the 'o' in hello changes the meaning of what one is trying to say.... |
| 07:45 |
Dyrcona |
I sometimes think we're doing ourselves a disservice with modern communications. |
| 07:58 |
|
collum joined #evergreen |
| 08:05 |
|
dbwells joined #evergreen |
| 08:07 |
|
mantis joined #evergreen |
| 08:44 |
|
mmorgan joined #evergreen |
| 08:44 |
|
alynn26 joined #evergreen |
| 09:13 |
|
rfrasur joined #evergreen |
| 09:15 |
|
dbwells joined #evergreen |
| 09:47 |
|
rfrasur joined #evergreen |
| 09:59 |
Dyrcona |
jeffdavis | jeff | miker: I think this thing we were looking at yesterday is a Perl bug. There should not be a difference between scalar @$array_ref and scalar @array, but there is. |
| 10:08 |
Dyrcona |
I see. It's more complicated than that. ... Perl sucks. |
| 10:10 |
jeff |
I agree that the change in behavior should be considered a Perl bug, but it's present in a wide enough range of Perl releases that we probably need to take steps (like the 0+scalar(@list) or int(scalar(@list)) options mentioned here) to work around it. Also, it's almost luck that we didn't hit something like this before with regard to JSON::XS mapping simple scalar values. |
| 10:10 |
Dyrcona |
It has something to do with certain contexts, like lists. On its own scalar @empty_array is a zero as a number. |
| 10:10 |
Dyrcona |
Yeah, I'm not saying that we shouldn't work around it. |
| 10:10 |
jeff |
> Simple Perl scalars (any scalar that is not a reference) are the most difficult objects to encode: JSON::XS will encode undefined scalars as JSON null values, scalars that have last been used in a string context before encoding as JSON strings, and anything else as number value |
| 10:10 |
jeff |
(from JSON::XS docs) |
| 10:11 |
jeff |
I saw mentioned elsewhere, but I think that documentation is also slightly wrong with regard to "last been used in a string context". |
| 10:11 |
Dyrcona |
Yes, but I don't think JSON::XS is our problem. I can make it happen without JSON::XS using my and the scalar in a list context. |
| 10:12 |
Dyrcona |
Yeah, the documentation is definitely wrong on that. I'll try a simple case again. |
| 10:13 |
jeff |
Right. This isn't a change in JSON:XS or a bug (other than perhaps a doc bug) in JSON:XS, it's a change in Perl (as of the 2017 commit I linked last night) which is resulting in scalar(@lexical) returning a simple scalar value that Perl internals treat as having a string representation (as we can see with Devel::Peek). |
| 10:14 |
jeff |
I'm still very curious about the Perl change and what other issues it may cause for us or others, and the fact that it's different for a lexical my @list vs non-lexical (is that even the right term?) @list. |
| 10:16 |
Dyrcona |
Scope is interesting because it can vary by programming language. I think Perl uses dynamic scope, generally, and then "my" scopes something to the current module, typically considered lexical scope. |
| 10:16 |
jeff |
But I lack the knowledge of Perl internals to understand the commit that introduced the change. It might be worth reporting as a bug / discussion on the Perl dev list, if only so that others can determine if it's Bad or just Annoying. :-) |
| 10:16 |
jeff |
And we should be on the lookout for other places where it can bite us. |
| 10:17 |
Dyrcona |
Yeahp. I started to open a bug this morning, "Perl Changes Bite Us, again." :) |
| 10:17 |
* Dyrcona |
checks the log from last night. |
| 10:17 |
|
stephengwills joined #evergreen |
| 10:17 |
jeff |
I don't know if there's a better way for us to be typing our JSON values. Trying to answer that question right now is probably not needed to fix this current manifestation of the issue, and might be overreacting. :-) |
| 10:20 |
Dyrcona |
Well, adding 0 or wrapping with a cast to int works in this specific case. Adding 0 is probably more general for decimals. |
| 10:37 |
Dyrcona |
jeff: https://perldoc.perl.org/perlapi#PL_sv_zero |
| 10:38 |
Dyrcona |
I haven't found it in the code, but I suspect it has trouble determining string context. |
| 10:48 |
Dyrcona |
Right. *Backs out of the weeds.* |
| 10:49 |
Dyrcona |
I've also been trying to find a non-JSON context to reproduce this behavior, but no luck so far. |
| 10:50 |
Dyrcona |
I'm getting a numeric zero. |
| 10:58 |
jeff |
My understanding of why that is: the difference in behavior / characteristics is mostly in the Perl internals, which aren't available to Perl code. Things like Devel::Peek, B::*, and JSON::XS are perl extensions written in C which have access to the Perl API and can inspect the internals. JSON::XS uses this to try and treat simple scalar values correctly, but acknowledges that it's difficult. |
| 11:00 |
Dyrcona |
Yeah, well I was quickly getting into the elephant grass trying to chase a path through the Perl code itself. |
| 11:00 |
jeff |
In order to write a test case for my git bisect last night, I needed to redirect stderr to a string, so that when I called Devel::Peek::Dump() I could examine the output. |
| 11:01 |
Dyrcona |
So, when are we gonna ditch Perl? :P |
| 11:02 |
jeff |
Interesting. I don't think that the new value of scalar(my $lexical) is just any "0" with a string representation. That would be a SV = PV as shown with perl -MDevel::Peek -e '$str = "0"; Dump($str);' |
| 11:03 |
Dyrcona |
Admittedly, I need to get better at reading Devel::Peek output. |
| 11:05 |
jeff |
The new behavior is a SV = PVNV, which may be one of Perl's internal representations of true/false. |
| 11:06 |
Dyrcona |
Yeah, I see that. |
| 11:06 |
Dyrcona |
Without my it's SV = IV. |
| 11:06 |
* jeff |
nods |
| 11:07 |
jeff |
perlguts documents the various SV types |
| 11:08 |
Dyrcona |
On perl 5.26, it's PV = IV with or without my. |
| 11:09 |
jeff |
I think the first priority is to fix / work around the identified issues, then do a little looking to see if we have any other similar issues, and as/if time allows, gather some test cases and bring them to the attention of Perl hackers to see if this is something that needs any action on their part (fixing a bug, documenting the change in behavior, etc). |
| 11:09 |
Dyrcona |
perl -MDevel::Peek -e '@ar = (); $str = scalar @ar; Dump($str);' |
| 11:10 |
jeff |
There is a lot of elephant grass to go around. :-) |
| 11:11 |
Dyrcona |
And, I meant SV = IV when I typoed, PV = IV above, just for the logs. |
| 11:19 |
jeffdavis |
I can open a Launchpad bug about the issue today and push a fix using int(scalar()) for our one currently-known instance. |
| 11:19 |
Dyrcona |
Hm. This is like an itch that I keep scrathing.. |
| 11:19 |
stephengwills |
Has anyone created a filter on staff search to hide deleted bibs? |
| 11:21 |
|
jihpringle joined #evergreen |
| 11:22 |
jeff |
stephengwills: hiding deleted bibs is a default, iirc. are you talking about "empty" bibs, where the bibs themselves are not deleted but they have no holdings (or their holdings have all been deleted)? |
| 11:23 |
jeff |
I think you need to set a flag (and reingest your deleted bibs if the flag was off) and pass a search argument (maybe #deleted?) to your search query to search for deleted bibs. |
| 11:24 |
jeff |
(You can retrieve deleted bibs by ID without needing all that, but not search for them.) |
| 11:25 |
Dyrcona |
I think the staf client includes deleted bibs, doesn't it? |
| 11:27 |
jeff |
not by default, not intentionally. |
| 11:27 |
jeff |
if you've turned it on, sure. |
| 11:27 |
jeff |
and there was at least one bug (fixed in 2018) where they were getting through: bug 1773832 |
| 11:27 |
pinesol |
Launchpad bug 1773832 in Evergreen 3.0 "Bib records that are both deleted and empty show in staff search" [Medium,Fix released] https://launchpad.net/bugs/1773832 |
| 11:28 |
|
collum joined #evergreen |
| 11:28 |
jeff |
maybe bug 1731281 also |
| 11:28 |
pinesol |
Launchpad bug 1731281 in Evergreen "asset.copy_vis_attr_cache upgrade script includes deleted items" [Medium,Fix released] https://launchpad.net/bugs/1731281 |
| 11:30 |
Dyrcona |
Yeah, I was checking the staff search code and didn't see it. |
| 11:31 |
Dyrcona |
More elephant grass. :) |
| 11:37 |
stephengwills |
ok thanks. yes, I am talking about empty bibs. we have too many of them overwelming the catalogers I think. Thanks for your clarifications I’ll dig further into it. |
| 11:38 |
jeff |
I would like to hide empty bibs from staff by default, unless staff specify (by setting or by search option) that they wish/need to see them, but we haven't looked into making that happen. |
| 11:38 |
jeff |
Our usual solution so far is to delete the empty bibs after X amount of time. |
| 11:38 |
Dyrcona |
We have a script to delete empty bibs after so many days. |
| 11:38 |
Dyrcona |
:) |
| 11:39 |
jeff |
There's little benefit that I'm aware of in having them show up for all staff -- the only staff who want to see them would in theory be folk attaching items. |
| 12:16 |
|
stephengwills joined #evergreen |
| 13:00 |
jeffdavis |
bug 1923076 - hopefully a coherent account of the scalar(@arr) thing |
| 13:00 |
pinesol |
Launchpad bug 1923076 in Evergreen "Incorrect typing when encoding some values as JSON" [Undecided,New] https://launchpad.net/bugs/1923076 |
| 13:02 |
|
sandbergja joined #evergreen |
| 13:05 |
Dyrcona |
jeffdavis++ |
| 13:05 |
Dyrcona |
I've seen the bug on Debian Buster, with Perl 5.28, too, but not reproduced with it Evergreen there. |
| 13:15 |
|
Cocopuff2018 joined #evergreen |
| 13:16 |
|
jamesrf joined #evergreen |
| 13:38 |
|
alynn26_away joined #evergreen |
| 13:46 |
|
mmorgan1 joined #evergreen |
| 13:53 |
|
jvwoolf joined #evergreen |
| 14:09 |
Dyrcona |
Because I could reproduce Lp 1923076 on Debian Buster, I targeted it back to 3.5. We've supported Buster for longer than that. |
| 14:09 |
pinesol |
Launchpad bug 1923076 in Evergreen 3.6 "Incorrect typing when encoding some values as JSON" [Medium,Confirmed] https://launchpad.net/bugs/1923076 |
| 14:11 |
jeffdavis |
I wasn't sure if I should add a pullrequest tag - there's a fix for that specific case but perhaps we want to look for more instances/get more info before committing anything? |
| 14:17 |
Dyrcona |
Yeah, I think we should look for more potential cases. I targeted the bug anyway, 'cause hopefully we'll have it fixed before the monthly point releases. If not, I'll roll it forward. It should probably he higher than Medium. |
| 14:21 |
Dyrcona |
No biggie. We only use scalar on 450 lines in Open-ILS/src/perlmods/. |
| 14:31 |
Dyrcona |
In most cases so far, we're either not running output through JSON, or we're doing scalar on an arrayref that is "cast" to array, which doesn't seem to cause the problem. |
| 14:54 |
Dyrcona |
Well, a grep -R suggests there are not any other problematic uses of scalar under perlmods. |
| 14:54 |
Dyrcona |
I'd be more confident if someone else had a look. |
| 15:04 |
miker |
jeff / Dyrcona: not that we should plan for @$aref to continue working as before, but, am I understanding correctly that that /is/ working normally? it's only real arrays with no elements that end up using PL_sv_zero, IOW |
| 15:06 |
miker |
I'm doing a quick scan for possible trouble spots |
| 15:09 |
|
jvwoolf1 joined #evergreen |
| 15:09 |
Dyrcona |
miker: Yeah, it looks like only real arrays. |
| 15:11 |
Dyrcona |
perl -MDevel::Peek -e 'my $list = []; my $count = scalar @$list; Dump($count);' Gives me SV=IV. |
| 15:46 |
Dyrcona |
miker++ |
| 15:48 |
miker |
oh, I should mention, I also excluded all uses where the return value of scalar() was used inline with a numberish operator, like addition or multiplication, which forces numeric context and should be safe (per 0+ working) |
| 15:48 |
miker |
I basically limited the list to where it was not obviously safe |
| 15:59 |
JBoyer |
miker, I'm not so sure about your "I've excluded uses that are only used in truthiness tests..." from lp 1923076 , I got the impression that simply subjecting them to the testing might be enough to twiddle some bits in their storage and make them look string-y enough for JSON::XS. |
| 15:59 |
pinesol |
Launchpad bug 1923076 in Evergreen 3.6 "Incorrect typing when encoding some values as JSON" [Medium,Confirmed] https://launchpad.net/bugs/1923076 |
| 16:00 |
JBoyer |
Though I've mostly been at the edges of this discussion, if that's not what's actually happening that's great. |
| 16:00 |
miker |
JBoyer: oh, I mean like "if (scalar(@foo))" and the like |
| 16:00 |
miker |
like, we don't keep the output of that call around for maybe later sending to a browser |
| 16:02 |
JBoyer |
Ah, ok, now I follow. |
| 16:02 |
JBoyer |
miker++ |
| 16:03 |
miker |
or where it was already wrapped in int() ... there were a couple of those |
| 16:07 |
gmcharlt |
as a general announcement: I'm planning on cutting 3.7-rc on Monday and the 3.7.0 release on Wednesday |
| 16:12 |
|
jvwoolf joined #evergreen |
| 16:26 |
|
akilsdonk_ joined #evergreen |
| 16:27 |
jeff |
miker: and just think, any times where we're testing an empty @lexical in a boolean context, it's ~314% faster because of the optimization present in Perl 5.28! |
| 16:32 |
|
mantis left #evergreen |
| 16:58 |
miker |
jeff++ |
| 17:17 |
|
mmorgan left #evergreen |
| 17:43 |
Dyrcona |
Correct > Fast. |
| 17:43 |
Dyrcona |
Perl-- |
| 18:01 |
pinesol |
News from qatests: Testing Success <http://testing.evergreen-ils.org/~live> |
| 19:32 |
|
sandbergja joined #evergreen |
| 21:27 |
|
JBoyer joined #evergreen |
| 22:05 |
jeffdavis |
Lots of symspell-related deadlocks trying to import records in parallel on a 3.7 system |
| 22:07 |
pastebot |
"jeffdavis" at 168.25.130.30 pasted "example of deadlock on importing MARC records in parallel" (14 lines) at http://paste.evergreen-ils.org/10134 |
