This is an archive of the discontinued LLVM Phabricator instance.

Differential D53123

Change the timestamp of llvmcache-foo file to meet the thinLTO prune policy
ClosedPublic

Authored by tejohnson on Oct 10 2018, 7:34 PM.

Details

Reviewers
ruiu
craig.topper
smaslov
Jianping
• espindola
LuoYuanke
rupprecht
Commits
rGc8325b4b597a: Change the timestamp of llvmcache-foo file to meet the thinLTO prune policy
rLLD345977: Change the timestamp of llvmcache-foo file to meet the thinLTO prune policy
rL345977: Change the timestamp of llvmcache-foo file to meet the thinLTO prune policy
Summary

The case may randomly fail if we test it with command "
while llvm-lit tools/lld/test/ELF/lto/cache.ll; do true; done". It is because the llvmcache-foo file is younger than llvmcache-349F039B8EB076D412007D82778442BED3148C4E and llvmcache-A8107945C65C2B2BBEE8E61AA604C311D60D58D6. But due to timestamp precision reason their timestamp is the same. Given the same timestamp, the file prune policy is to remove bigger size file first, so mostly foo file is removed for its bigger size. And the files size is under threshold after deleting foo file. That's what test case expect.

However sometimes, the precision is enough to measure that timestamp of llvmcache-349F039B8EB076D412007D82778442BED3148C4E and llvmcache-A8107945C65C2B2BBEE8E61AA604C311D60D58D6 are smaller than foo, so llvmcache-349F039B8EB076D412007D82778442BED3148C4E and llvmcache-A8107945C65C2B2BBEE8E61AA604C311D60D58D6 are deleted first. Since the files size is still above the file size threshold after deleting the 2 files, the foo file is also deleted. And then the test case fails, because it expect only one file should be deleted instead of 3.

The fix is to change the timestamp of llvmcache-foo file to meet the thinLTO prune policy.
The same fix is applied to llvm code at https://reviews.llvm.org/D52452.

Diff Detail

Repository
rL LLVM

Event Timeline

LuoYuanke created this revision.Oct 10 2018, 7:34 PM
Herald added a reviewer: • espindola. · View Herald TranscriptOct 10 2018, 7:34 PM
Herald added subscribers: llvm-commits, dexonsmith, steven_wu and 3 others. · View Herald Transcript
tejohnson accepted this revision.Oct 15 2018, 8:55 AM

LGTM

This revision is now accepted and ready to land.Oct 15 2018, 8:55 AM
tejohnson added a comment.Oct 22 2018, 5:57 PM

ping on commit. Also - could you apply the same fix to tools/lld/test/wasm/lto/cache.ll?

bjope added a subscriber: bjope.Oct 23 2018, 8:36 AM
rupprecht added a subscriber: rupprecht.Oct 30 2018, 11:47 AM

Seconding the ping to commit -- I'm still regularly seeing buildbot failures for this test.

http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/13622/steps/test-stage4-compiler/logs/stdio

tejohnson commandeered this revision.Oct 30 2018, 11:56 AM
tejohnson edited reviewers, added: LuoYuanke; removed: tejohnson.

I'll go ahead and take this over. Will upload a new version with the wasm/lto/cache.ll fix.

This revision now requires review to proceed.Oct 30 2018, 11:56 AM
tejohnson updated this revision to Diff 171757.Oct 30 2018, 12:13 PM
tejohnson edited the summary of this revision. (Show Details)

Add the same fix to wasm/lto/cache.ll

Herald added subscribers: aheejin, sbc100. · View Herald TranscriptOct 30 2018, 12:13 PM
Harbormaster completed remote builds in B24344: Diff 171757.Oct 30 2018, 12:13 PM
tejohnson added a comment.Nov 1 2018, 2:35 PM

ping

dexonsmith removed a subscriber: dexonsmith.Nov 1 2018, 3:03 PM
LuoYuanke added a comment.Nov 1 2018, 6:08 PM

Hi Johnson,

Sorry for response too late. The patch looks good to me. Is there any process we need follow before check in the patch?

Thanks
Yuanke

tejohnson added a comment.Nov 2 2018, 7:22 AM
In D53123#1285005, @LuoYuanke wrote:

Hi Johnson,

Sorry for response too late. The patch looks good to me. Is there any process we need follow before check in the patch?

Thanks
Yuanke

Thanks - just needs someone to mark the patch approved. Teresa

rupprecht accepted this revision.Nov 2 2018, 7:39 AM
This revision is now accepted and ready to land.Nov 2 2018, 7:39 AM
Closed by commit rL345977: Change the timestamp of llvmcache-foo file to meet the thinLTO prune policy (authored by tejohnson). · Explain WhyNov 2 2018, 7:54 AM
This revision was automatically updated to reflect the committed changes.
tejohnson added a comment.Nov 2 2018, 8:25 AM

Had to revert:

commit edd04e964e15a71cad6786ec940475a837baabec (HEAD -> master, origin/master, origin/HEAD)
Author: tejohnson <tejohnson@91177308-0d34-0410-b5e6-96231b3b80d8>
Date: Fri Nov 2 15:22:19 2018 +0000

Revert "Change  the timestamp of llvmcache-foo file to meet the thinLTO prune policy"

This reverts commit r345977.

A few bots failing because the invocation of "touch" is not accepted on
a couple other OSes. Specifically the -d argument is not accepted or requires
a different format.

http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/27103/steps/test_lld/logs/stdio
http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/24974/steps/test_lld/logs/stdio

git-svn-id: https://llvm.org/svn/llvm-project/lld/trunk@345980 91177308-0d34-0410-b5e6-96231b3b80d8

Will need some investigation to figure out if there is a universally accepted invocation of touch that will do the trick.

MaskRay added a subscriber: MaskRay.Nov 2 2018, 10:24 AM
In D53123#1285506, @tejohnson wrote:

Had to revert:

commit edd04e964e15a71cad6786ec940475a837baabec (HEAD -> master, origin/master, origin/HEAD)
Author: tejohnson <tejohnson@91177308-0d34-0410-b5e6-96231b3b80d8>
Date: Fri Nov 2 15:22:19 2018 +0000

Revert "Change  the timestamp of llvmcache-foo file to meet the thinLTO prune policy"

This reverts commit r345977.

A few bots failing because the invocation of "touch" is not accepted on
a couple other OSes. Specifically the -d argument is not accepted or requires
a different format.

http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/27103/steps/test_lld/logs/stdio
http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/24974/steps/test_lld/logs/stdio

git-svn-id: https://llvm.org/svn/llvm-project/lld/trunk@345980 91177308-0d34-0410-b5e6-96231b3b80d8

Will need some investigation to figure out if there is a universally accepted invocation of touch that will do the trick.

https://reviews.llvm.org/D54039 :)

Revision Contents

PathSize
lld/
trunk/
test/
ELF/
lto/
4 lines
wasm/
lto/
4 lines

Diff 172358

lld/trunk/test/ELF/lto/cache.ll

Show All 13 Lines
; Create a file of size 64KB. ; Create a file of size 64KB.
; RUN: %python -c "print(' ' * 65536)" > %t.cache/llvmcache-foo ; RUN: %python -c "print(' ' * 65536)" > %t.cache/llvmcache-foo
; This should leave the file in place. ; This should leave the file in place.
; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=128k:prune_interval=0s -o %t3 %t2.o %t.o ; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=128k:prune_interval=0s -o %t3 %t2.o %t.o
; RUN: ls %t.cache | count 5 ; RUN: ls %t.cache | count 5
; Increase the age of llvmcache-foo, which will give it the oldest time stamp
; so that it is processed and removed first.
; RUN: touch -r %t.cache/llvmcache-foo -d '-2 minutes' %t.cache/llvmcache-foo
; This should remove it. ; This should remove it.
; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=32k:prune_interval=0s -o %t3 %t2.o %t.o ; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=32k:prune_interval=0s -o %t3 %t2.o %t.o
; RUN: ls %t.cache | count 4 ; RUN: ls %t.cache | count 4
; Setting max number of files to 0 should disable the limit, not delete everything. ; Setting max number of files to 0 should disable the limit, not delete everything.
; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=0s:cache_size=0%:cache_size_files=0:prune_interval=0s -o %t3 %t2.o %t.o ; RUN: ld.lld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=0s:cache_size=0%:cache_size_files=0:prune_interval=0s -o %t3 %t2.o %t.o
; RUN: ls %t.cache | count 4 ; RUN: ls %t.cache | count 4
Show All 25 Lines

lld/trunk/test/wasm/lto/cache.ll

Show All 11 Lines
; Create a file of size 64KB. ; Create a file of size 64KB.
; RUN: %python -c "print(' ' * 65536)" > %t.cache/llvmcache-foo ; RUN: %python -c "print(' ' * 65536)" > %t.cache/llvmcache-foo
; This should leave the file in place. ; This should leave the file in place.
; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=128k:prune_interval=0s -o %t.wasm %t2.o %t.o ; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=128k:prune_interval=0s -o %t.wasm %t2.o %t.o
; RUN: ls %t.cache | count 5 ; RUN: ls %t.cache | count 5
; Increase the age of llvmcache-foo, which will give it the oldest time stamp
; so that it is processed and removed first.
; RUN: touch -r %t.cache/llvmcache-foo -d '-2 minutes' %t.cache/llvmcache-foo
; This should remove it. ; This should remove it.
; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=32k:prune_interval=0s -o %t.wasm %t2.o %t.o ; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy cache_size_bytes=32k:prune_interval=0s -o %t.wasm %t2.o %t.o
; RUN: ls %t.cache | count 4 ; RUN: ls %t.cache | count 4
; Setting max number of files to 0 should disable the limit, not delete everything. ; Setting max number of files to 0 should disable the limit, not delete everything.
; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=0s:cache_size=0%:cache_size_files=0:prune_interval=0s -o %t.wasm %t2.o %t.o ; RUN: wasm-ld --thinlto-cache-dir=%t.cache --thinlto-cache-policy prune_after=0s:cache_size=0%:cache_size_files=0:prune_interval=0s -o %t.wasm %t2.o %t.o
; RUN: ls %t.cache | count 4 ; RUN: ls %t.cache | count 4
Show All 11 Lines