Manifest path trie RFC #40
Conversation
|
If we have performance numbers for this adding them to the RFC would help make the case for accepting this change. The details section could mention more clearly the benefits other than file size e.g. the ability to support partial updates. |
rfcs/manifest-path-trie.md
Outdated
|
|
||
| The proposal here is two-fold: | ||
|
|
||
| Firstly, replace the items objects with a series of nested objects forming a trie of path segments. This makes it much quicker to iterate through only specific directories, which is often enough done in a test-debug cycle. |
This RFC itself makes little difference to performance; the big gain is it makes include/exclude much easier to reimplement without iterating through all tests. The only significant change here is it reduces serialization cost, which is primarily apparent when a small number of tests are modified.
This isn't actually any different than before, and I don't think it's changed the implementation complexity of that (reftest-simplification did that). |
|
This essentially implements web-platform-tests/rfcs#40.
|
So yay, I wasn't aware of this change, which caused all of Bikeshed's WPT data to become incorrect and so the entire I can monitor this repo if necessary, but I wasn't even aware that it existed - it's not, to my knowledge, linked from the README in the main /wpt repo. Somewhat annoyed that Bikeshed, a known major consumer of the manifest data, wasn't notified of a major change to the manifest format. |
|
@tabatkins we're terribly sorry. This really should not have happened. I'm not sure why Bikeshed didn't cross anyone's mind... Especially when I filed web-platform-tests/wpt.fyi#1777 (I even said "all features that rely on /api/manifest are off by default", sigh...) We are taking steps to prevent this from happening. In addition to the making the RFC repo more prominent from the main WPT repo and including the version number in the response headers of |
|
So first of all, apologies for breaking bikeshed; that was obviously unintentional. I had no idea that it was using the manifest, and to be honest didn't even remember that wpt.fyi was providing a public API here. So clearly one thing that would have improved the situation would be having more consumers pay attention to the RFC repo. But I honestly doubt linking it in the README — though certainly a good idea — is going to make much difference. The RFC process was for example announced on the mailing list, and rather extensively discussed on IRC. It seems unlikely someone who missed all of that would notice a link buried in an already extensive README. Even if you knew about the RFC repo, it technically doesn't cover wpt.fyi endpoints, and so it would be reasonable to assume that as a consumer of those endpoints changes in the RFC repo don't apply to you. So in addition to the question about how to make people more aware of RFCs, I think we need to have a clear understanding of what stability guarantees are supposed to be offered by wpt.fyi endpoints, and ensure that there's some mechanism to get notified of changes there. It seems possible that the expectations around stability of the manifest format (largely: that it isn't) are incompatible with the kind of guarantees that wpt.fyi might want to make in general, so perhaps we should only make the raw manifest available on a URL that's explicitly marked unstable, and make other data from the manifest available in a stable format. |
|
Apologies accepted, it was a minor hiccup overall and quickly fixed.
I just did a search thru my email for "wpt rfc" and didn't see any relevant hits. When/where did this discussion happen?
w3.org#testing? That channel is extremely noisy with BitBot announcements; if I catch any discussion at all I consider myself lucky. Right now, my 30' vertical-oriented screen shows messages in that room up to 2am this morning (~11 hours ago)
Strong disagree. There's a nice list of links already in the README with explanations of why you might want to visit each one; having one linking to /rfcs/ and explaining that breaking changes get announced/discussed there would have been caught by me, at least. |
|
(That said, I wouldn't mind a more dedicated mailing list for this kind of thing.) |
…, a=testonly Automatic update from web-platform-tests Major new manifest version (v8): path trie edition This essentially implements web-platform-tests/rfcs#40. -- wpt-commits: 31c0f5efba38b7d1d7f45ac449bcbc892e8771ce wpt-pr: 16537
…, a=testonly Automatic update from web-platform-tests Major new manifest version (v8): path trie edition This essentially implements web-platform-tests/rfcs#40. -- wpt-commits: 31c0f5efba38b7d1d7f45ac449bcbc892e8771ce wpt-pr: 16537 UltraBlame original commit: a35a8d5c00bdfb36d38d996eae17921ad8495154
…, a=testonly Automatic update from web-platform-tests Major new manifest version (v8): path trie edition This essentially implements web-platform-tests/rfcs#40. -- wpt-commits: 31c0f5efba38b7d1d7f45ac449bcbc892e8771ce wpt-pr: 16537 UltraBlame original commit: a35a8d5c00bdfb36d38d996eae17921ad8495154
…, a=testonly Automatic update from web-platform-tests Major new manifest version (v8): path trie edition This essentially implements web-platform-tests/rfcs#40. -- wpt-commits: 31c0f5efba38b7d1d7f45ac449bcbc892e8771ce wpt-pr: 16537 UltraBlame original commit: a35a8d5c00bdfb36d38d996eae17921ad8495154
…, a=testonly Automatic update from web-platform-tests Major new manifest version (v8): path trie edition This essentially implements web-platform-tests/rfcs#40. -- wpt-commits: 31c0f5efba38b7d1d7f45ac449bcbc892e8771ce wpt-pr: 16537
|
The list where this was discussed is public-test-infra: https://lists.w3.org/Archives/Public/public-test-infra/2019JanMar/0002.html However, I'm not surprised that not everyone who could be affected by changes to WPT are on that list. Anyway, looks like things got sorted pretty quickly here, and web-platform-tests/wpt#21682 updated the README. |
|
Yup, I wasn't subscribed to that; I am now. And yeah, the README change is good; I consider my issue dealt with. Thanks, all! |
Rendered: https://github.com/gsnedders/rfcs/blob/manifest-path-trie/rfcs/manifest-path-trie.md.
See also #20 and web-platform-tests/wpt#16537.