Ticket #1544 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

t/profiling/profiling.t fails on darwin

Reported by: dukeleto Owned by: jkeenan
Priority: normal Milestone:
Component: testing Version: 2.2.0
Severity: high Keywords:
Cc: Language:
Patch status: Platform: darwin

Description

$ prove -v ./t/profiling/profiling.t 
./t/profiling/profiling.t .. 
1..12
ok 1 # profile creation didn't explode
ok 2 # profile has a version number
ok 3 # profile contains a CLI string
ok 4 # profile has a say op
ok 5 # profile has canonical timing information
ok 6 # matcher didn't find non-existent opcode
ok 7 # profile shows 'say' inside main sub
not ok 8 # profile properly reflects normal control flow (simple)
not ok 9 # profile properly reflects normal control flow (slightly less simple)
not ok 10 # profile properly reflects tailcall control flow
ok 11 # profile shows say on the correct line
not ok 12 # profile shows 'say' inside nqp sub
Failed 4/12 subtests 

Test Summary Report
-------------------
./t/profiling/profiling.t (Wstat: 0 Tests: 12 Failed: 4)
  Failed tests:  8-10, 12
Files=1, Tests=12,  1 wallclock secs ( 0.03 usr  0.01 sys +  0.99 cusr  0.19 csys =  1.22 CPU)
Result: FAIL

Change History

follow-up: ↓ 2   Changed 4 years ago by Benabik

Using git-svn, I bisected the problem to r45337: "[proftest] use more generic, if predictable, temp files". Reverting it allows the test to complete. My guess is that it's related to how %config<tempdir> is (or isn't) set on Darwin, since my quick search doesn't show any other use of it.

in reply to: ↑ 1   Changed 4 years ago by coke

Replying to Benabik:

Using git-svn, I bisected the problem to r45337: "[proftest] use more generic, if predictable, temp files". Reverting it allows the test to complete. My guess is that it's related to how %config<tempdir> is (or isn't) set on Darwin, since my quick search doesn't show any other use of it.

Good call. on darwin, my tempdir in config_lib.pasm is set to:

    set P0["tempdir"], "/var/folders/Bh/BhrBssnnHYqX6SDI8N3S1U+++TM/-Tmp-"

Which looks like its intended to be a one-time use tempdir, not something that we can save and reuse elsewhere. Looks like it's set here:

config/init/defaults.pm
244:        tempdir => File::Spec->tmpdir,

If we intend to reuse this tempdir outside of config time, we need to fix this setting. FYI, I can duplicate the test failure; If I change my tempdir in config_lib.pasm to "/tmp", the test passes.

follow-ups: ↓ 4 ↓ 5   Changed 4 years ago by jkeenan

I am unable to reproduce this bug on Darwin/PPC, Mac OS X 10.4.11.

$ perl -MFile::Spec -e 'print File::Spec->tmpdir(), "\n";'
/tmp

$ grep -ni tempdir lib/Parrot/Config/Generated.pm config_lib.pasm
lib/Parrot/Config/Generated.pm:350:  'tempdir' => '/tmp',
config_lib.pasm:257:    set P0["tempdir"], "/tmp"

$ prove -v t/profiling/profiling.t
1..12
ok 1 # profile creation didn't explode
ok 2 # profile has a version number
ok 3 # profile contains a CLI string
ok 4 # profile has a say op
ok 5 # profile has canonical timing information
ok 6 # matcher didn't find non-existent opcode
ok 7 # profile shows 'say' inside main sub
ok 8 # profile properly reflects normal control flow (simple)
ok 9 # profile properly reflects normal control flow (slightly less simple)
ok 10 # profile properly reflects tailcall control flow
ok 11 # profile shows say on the correct line
ok 12 # profile shows 'say' inside nqp sub
ok
All tests successful.
Files=1, Tests=12, 13 wallclock secs 
  ( 0.10 usr  0.04 sys +  8.84 cusr  0.99 csys =  9.97 CPU)
Result: PASS

I am inclined to suspect that either (a) something has changed about more recent versions of Darwin re Perl 5; or (b) something is peculiar about these particular boxes.

Given how many of our developers have worked on Darwin over the years, I think we would have encountered this bug long before now. So I'm skeptical as to whether it constitutes a real bug.

Thank you very much.

kid51

P.S.: We could really benefit from Smolder reports on Darwin/i386. We used to have a lot of those, but lately the only Darwin Smolder reports have been from my 6-year-old iBook.

in reply to: ↑ 3 ; follow-up: ↓ 6   Changed 4 years ago by Benabik

Replying to jkeenan:

I am unable to reproduce this bug on Darwin/PPC, Mac OS X 10.4.11. I am inclined to suspect that either (a) something has changed about more recent versions of Darwin re Perl 5; or (b) something is peculiar about these particular boxes.

Something has changed between Tiger (10.4) and Snow Leopard (10.6).

A few more moments of poking shows me that my $ENV{TMPDIR} is set to /var/folders/rn/rnynkgxVGVaPI61zM32Nx++++TI/-Tmp-/ This is stable across processes, but I have yet to see if it will change for different login sessions or reboots. I'm guessing launchd creates a new per-user temp dir at login. The problem here is that we're caching TMPDIR and there's no guarantee that it will be stable.

Instead of reverting r45337, I guess I'll set TMPDIR to /tmp when I compile. But that falls into the realm of "workaround" and not "solution". Parrot should either rely on /tmp again or check $ENV{TMPDIR} on startup instead of compile.

P.S.: We could really benefit from Smolder reports on Darwin/i386. We used to have a lot of those, but lately the only Darwin Smolder reports have been from my 6-year-old iBook.

I'll replace "make test" with "make smoke" in my standard build script. I tend to rebuild Parrot every few days, so this should help out.

in reply to: ↑ 3   Changed 4 years ago by Benabik

And I forgot to respond to this:

Replying to jkeenan:

Given how many of our developers have worked on Darwin over the years, I think we would have encountered this bug long before now. So I'm skeptical as to whether it constitutes a real bug.

It's just that r45337 is the first use of %config<tempdir>. Parrot uses File::Spec->tempdir in perl scripts plenty, but never before the cached version stored in the global config hash. So this has been broken for a long time, but never before used.

in reply to: ↑ 4 ; follow-up: ↓ 7   Changed 4 years ago by jkeenan

Replying to Benabik:

Replying to jkeenan: A few more moments of poking shows me that my $ENV{TMPDIR} is set to /var/folders/rn/rnynkgxVGVaPI61zM32Nx++++TI/-Tmp-/

That is different. Yuck!

We could, of course, offer a command-line option, say, something like:

--tmpdir=/tmp

... which would override the directory chosen by File::Spec->tmpdir() on a given platform. Would that work for you?

kid51

in reply to: ↑ 6   Changed 4 years ago by coke

Replying to jkeenan:

Replying to Benabik:

Replying to jkeenan: A few more moments of poking shows me that my $ENV{TMPDIR} is set to /var/folders/rn/rnynkgxVGVaPI61zM32Nx++++TI/-Tmp-/

That is different. Yuck! We could, of course, offer a command-line option, say, something like: {{{ --tmpdir=/tmp }}} ... which would override the directory chosen by File::Spec->tmpdir() on a given platform. Would that work for you? kid51

No. we shouldn't be caching the value obtained during Configure.pl for use at parrot runtime.

  Changed 4 years ago by christoph@…

Parrot wrote:
> #1544: t/profiling/profiling.t fails on darwin
> ----------------------+-----------------------------------------------------
>  Reporter:  dukeleto  |       Owner:       
>      Type:  bug       |      Status:  new  
>  Priority:  normal    |   Milestone:       
> Component:  testing   |     Version:  2.2.0
>  Severity:  high      |    Keywords:       
>      Lang:            |       Patch:       
>  Platform:  darwin    |  
> ----------------------+-----------------------------------------------------
> 
> Comment(by coke):
> 
>  Replying to [comment:6 jkeenan]:
>  > Replying to [comment:4 Benabik]:
>  > > Replying to [comment:3 jkeenan]:
>  > >
>  > > A few more moments of poking shows me that my `$ENV{TMPDIR}` is set to
>  `/var/folders/rn/rnynkgxVGVaPI61zM32Nx++++TI/-Tmp-/`
>  >
>  > That '''is''' different.  Yuck!
>  >
>  > We could, of course, offer a command-line option, say, something like:
>  > {{{
>  > --tmpdir=/tmp
>  > }}}
>  > ... which would override the directory chosen by File::Spec->tmpdir() on
>  a given platform.  Would that work for you?
>  >
>  > kid51
> 
>  No. we shouldn't be caching the value obtained during Configure.pl for use
>  at parrot runtime.
> 

For clarification, should code be using config['tempdir'] at all or is there 
some (nyi?) function that should be used instead?

  Changed 3 years ago by jkeenan

The following suggests that we are only using config['tempdir'] in one file:

$ find . -type f | xargs grep -n 'config.*tempdir'
./runtime/parrot/library/ProfTest/PIRProfile.nqp:66:    my $tmp_pir   := %config<tempdir> ~ %config<slash> ~ 'test.pir';
./runtime/parrot/library/ProfTest/PIRProfile.nqp:67:    my $tmp_pprof := %config<tempdir> ~ %config<slash> ~ 'test.pprof';

Could we eliminate it?

Thank you very much.

kid51

follow-up: ↓ 11   Changed 3 years ago by jkeenan

In t/profiling/profiling.t I am getting no test errors and I see no todo-ed tests. So it would seem that the original issues raised in this ticket has been resolved.

If so, then perhaps we should move the discussion about `configtempdir? to another ticket and close this one.

Thank you very much.

kid51

in reply to: ↑ 10   Changed 3 years ago by jkeenan

  • owner set to jkeenan

Replying to jkeenan:

In t/profiling/profiling.t I am getting no test errors and I see no todo-ed tests. So it would seem that the original issues raised in this ticket has been resolved. If so, then perhaps we should move the discussion about `configtempdir? to another ticket and close this one.

Hearing no further complaints about t/profiling/profiling.t, I am closing this ticket and have opened TT #2085 to discuss the tmpdir issues.

Thank you very much.

kid51

  Changed 3 years ago by jkeenan

  • status changed from new to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.