backtrack if can not activate

This is a fix for #4347
Unfortunately this too regressed error messages for the case that you specified a dependency feature that does not  exist.
@alexcrichton advice on improving the message?

r? @alexcrichton
(rust_highfive has picked a reviewer for you, use r? to override)

Oh dear that's a bummer! It seems like a lot of roads are pointing towards error messages not being that great...
This one I would say though yeah is even more pressing than links in that this comes up quite frequently (typos and such). In that sense it'd be pretty bad if we regressed this error message :(

Is there some way to print a error just as we would have but then continue
processing, if so I have a bunch of ideas for how to improve error
messages, at least in this case.
Or is there some way to tie an an error message to a previous one. So we
can say options for dependent b causes include...
On Feb 2, 2018 6:10 PM, "Alex Crichton"<EMAIL_ADDRESS>wrote:

Oh dear that's a bummer! It seems like a lot of roads are pointing towards
error messages not being that great...
This one I would say though yeah is even more pressing than links in that
this comes up quite frequently (typos and such). In that sense it'd be
pretty bad if we regressed this error message :(
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/cargo/pull/5000#issuecomment-362735995, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADiaQDfACrexbHlxNdx5FQGjornubYrVks5tQ5XxgaJpZM4R2Pge
.

It will take some work to rebase/merge this with #4834, and that is a much bigger/more common UX improvement. I think this should be put on hold for that to land and for a plan for error messages to be developed.
@alexcrichton You are right, I have been thoroughly convinced that error messages should happen next. You offered some mentership on that, how do you think I should proceed?

@Eh2406 certainly! Unfortunately I don't really know how to precisely proceed. I do have a feeling, though, for how to proceed generally!
So prior to https://github.com/rust-lang/cargo/pull/4978 we had a custom error message for the links key which indicated. I think that a great end state would be to end up basically as close as we can to an error message like that where you've got a causal chain through packages and a nice description for what's happening.
As to how to get there... I'm not sure. I think it may also be worth taking a step back and looking at the resolution algorithm from a distance too. Right now Cargo in theory attempts all possible crate resolution graphs. I mean like all possible graphs. This can put us in one of two situations:

At least one graph is "successful" and satisfies all the constraints. In this situation Cargo should never return an error and eventually reach such a graph.
All possible graphs would lead to a resolution failure, so Cargo should return an error.

Here we're most interested in the latter situation, but it also means there's no one error for us to return. In some sense we tried everything and it could have all failed for a number of reasons. I think we could probably optimize to return the "shortest" error perhaps? (for whatever definition "shortest" actually means)
There's also the separate bug that Cargos' backtracking and exploration of the problem space takes forever, but hopefully https://github.com/rust-lang/cargo/pull/4834 can help with that!

Hm so that's unfortunately not super actionable, but is that helpful at least to get started?

Thanks for the pointer to the links code, I will go and read that. I like the idea of having a good causal chain for the cases when we can.
I think it may be useful to break it down into more situations.

The tree is trivially solvable, even if it is big. This is common, For example Servo, when I tested it, does No backtracking at all. -> No error messages.
The tree is solvable, given a exponential amount of time. #4834 will make it much more rare, But the problem is NP complete so this is always a possibility. -> We should print some kind of diagnostic to help the programer make it easier for us. Ether at some time into resolution like we do Resolving dependency graph... Or wen we bail after a user configured number of time as you suggested in #4066
The tree has no solutions. -> We need to report something useful to the programer.

We should keep in mind that the most common reason for any kind of error is a simple typo.
And that the user's mental model does not include millions of permutations of ancient versions of the dependencies.
The first time we discover that the tree is not trivial we are in the state closest to the user's mental model of just take the newest version of everything. And almost anything we print will be helpful in discovering that it is just a typo.  So I think we should print a good causal chain for the backtracking to the user, and continue in the serch.
Another huristick if something deep in our tree is unsolvable, then we probably hit almost the same error message lots of times. For example We depend on A=* (with 100 versions to choose from) which depends on B=* (with 100 versions to choose from) which depends on C=* all versions have been yanked, If we printed all errors (a thout experiment) We will have 10k "can't find a candidate for C ...." and just 100 "can't find a candidate for B ...." and 1 "can't find a candidate for A ...." The most common one is the most helpful. So maybe 10 minutes into resolution we can print "we are still working on finding something that works but, here are the 3 most often encountered difficulties:" each with a full a good causal chain for the backtracking.
All of these thoughts depend on being able to print the message as part of some context, and then continue with the serch. How do I do that? I could make a function that returns the errors as strings. But the words "errors as strings" makes me very nervous.

All excellent points! I think we should take it as an assumption that if we're going to produce a human readable error then we can reach that conclusion in a reasonable amount of time. In that sense I hope that #4834 will get us closer to that goal but we may have other bugs of "Cargo goes in the weeds too much". I don't think, though, that such a situation should impact the design of the error messages here.
Now I will say we have two other tricks up our sleeve for "gee resolution is taking forever":

First we can actually inform the user what's happening. Recently Cargo changed to do this and will print out, after I think a second or so of resolution, "Resolving crate graph...". It's not much but it at least let's the user know we're not stuck in network traffic.
Second we should probably implement Cargo just giving up at some point. If Cargo has attempted a million graphs and they're all failing, it's doubtful that the million+1th graph will work. This is sort of covered by https://github.com/rust-lang/cargo/issues/4066 where we should (a) have a hard limit on the amount of time Cargo takes during resolution and (b) when this error happens mention something like "hey this may be a bug in Cargo, just letting you know"

In that sense I think your second and third cases would fold into one another. I definitely agree that mistakes tend to be common typos before we get to serious "oh dear what to do" scenarios. Despite that though if Cargo is indeed successful I don't think that we should print out anything (even if we hit errors along the way). I think that only if we end up failing entirely should we print an error.
We could certainly explore though a more progress-bar like situation for Cargo where it prints incrementally what it's doing for debugging perhaps?

All of these thoughts depend on being able to print the message as part of some context, and then continue with the serch. How do I do that? I could make a function that returns the errors as strings. But the words "errors as strings" makes me very nervous.

I think it's sort of encoded as CargoResult or weird options in resolve today, but I think we should stick with that. An error happening only happens in the "slow" case where "slow" means that you've modified or don't already have a Cargo.lock. On the fast path we'll never hit an error because the Cargo.lock means we'll resolve precisely to the same one as before.
In that sense I wouldn't be too worried about making errors expensive to compute, it hopefully isn't too bad! We can of course profile and test on Servo though to test this out.

Ok, I think we mostly agree and if we don't it is because we are talking past each other. I think I need to start thinking more "incrementally". I should start with reading the old 'links' code and seeing if I can make the current error messages better. Only then come back to the larger questions of when to print and for witch backtracks.

I think this is finally ready to go.
Some things I did not figure out how to test:

Where to clone BacktrackFrame. I think I have this right. I think my initial version could have had O(n^2), but I could not get a test case that proved it.
Adding conflicting_activations to the BacktrackFrame. I think this is needed as I described in the commit, but I could not get a test case that proved it.

Nah this looks great to me, thanks @Eh2406
@bors: r+

:pushpin: Commit 2cbd1dd has been approved by alexcrichton

:hourglass: Testing commit 2cbd1ddc93024abc752355a8b9efcf4f92aaccf7 with merge 382967ad10e9315513bed959bda9d9d866328e83...

:sunny: Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 382967ad10e9315513bed959bda9d9d866328e83 to master...


feat: add commit lints for consistency

@lwhiteley will the husky hook catch my bad commit messages and block them for me?

@lwhiteley will the husky hook catch my bad commit messages and block them for me?

yes it will

:+1: :apple: approved

can you also only allow 'squash and merge' pull request acceptance?


Company

Create company structure.

[ ] Models
[ ] Admin panel
[ ] Fixtures

Trading crypto and making profit, an expert like Lauren B Alex will guide you to make good profits,  she is on Facebook.


Add out-of-order sample support to the TSDB

This implementation is based on this design doc:
https://docs.google.com/document/d/1Kppm7qL9C-BJB1j6yb6-9ObG3AbdZnFUBYPNNWwDBYM/edit?usp=sharing
This commit adds support to accept out-of-order ("OOO") sample into the TSDB
up to a configurable time allowance. If OOO is enabled, overlapping querying
are automatically enabled.

This replaces https://github.com/grafana/mimir-prometheus/pull/268. Somehow having a single commit while manually adding co-authors removes the CLA requirement from the co-authors. (See 268, CLA fails because Dieter hasn't signed, but it does not fail here).

We can consider merging this after https://github.com/grafana/mimir-prometheus/pull/270/ is merged and synced in this PR.

Lets get the OOOAllowance and the OOO WBL dir rename into this PR.

Opened https://github.com/grafana/mimir-prometheus/pull/273 for TODOs

This PR changes ChunkReader interface in a way that's incompatible with usage in Thanos and Mimir. We could fix Mimir, but unless you plan to vendor mimir-prometheus in Thanos, it needs to be reverted :(

Did this mean TSDB that supports out of order data is not compatible with thanos? So I can't use it in cortex v1.11 that supports obs storage?

@nwpuCfy that comment was about code interface and not the data format. Thanos has switched to the new interface I think. And they use prometheus/prometheus, and not grafana/mimir-prometheus.

@nwpuCfy As @codesome pointed out, thanos is now vendoring prometheus/prometheus were we ported the changes you see in this PR so there should not be any problems :+1:  Use prometheus/prometheus as your reference for the official state of out-of-order support.

@nwpuCfy As @codesome pointed out, thanos is now vendoring prometheus/prometheus were we ported the changes you see in this PR so there should not be any problems 👍 Use prometheus/prometheus as your reference for the official state of out-of-order support.
ok, thanks for reply

@nwpuCfy that comment was about code interface and not the data format. Thanos has switched to the new interface I think. And they use prometheus/prometheus, and not grafana/mimir-prometheus.

thanks for reply


(Continuous) Batch Serving Internvl2 using lmdeploy

I'm trying to serve InternVL2 (llama3 76B) using lmdeploy as the example here on 4 A100-80G GPUs. However, I found that the server is always processing one request after another.
I've referred to Lmdeploy's Issue to adjust the --vision-max-batch-size to 8, but still can't see the server processing requests in parallel.
Is it possible to server a InternVL2-Llama3-76B where (dynamic) batching is enabled? By batching, I mean the server can generate response for multiple request at the same time (probably similar to vllm's continuous batching). If so, how to do it?
If not possible, is there someway that I can process batched inputs in one request, i.e. requesting 4 (or 8 etc.) different outputs together in one request?

You can use batch inference, or you can deploy an openai api server and then request it.

Thanks, I've understood the whole process through lmdeploy's issue

@zzjchen Were you able to achieve the above issue raised? Processing multiple requests at same time? If yes can how did u do it . Please let me know.


Line typing

I think I should have ran the tests on a different version of the file, sorry for that, so ignore my ping haha
Tomorrow morning I'll fix it!

hey @martinRenou, thanks for the help! So, as I said, a couple of weird bugs. One of them is this kind:
    src/Lines.ts(256,42): error TS2769: No overload matches this call.
      Overload 1 of 2, '(data: [number, number][]): string', gave the following error.
        Argument of type '[number[]]' is not assignable to parameter of type '[number, number][]'.
          Type 'number[]' is missing the following properties from type '[number, number]': 0, 1
      Overload 2 of 2, '(data: [number, number][]): void', gave the following error.
        Argument of type '[number[]]' is not assignable to parameter of type '[number, number][]'.

Complaining about the type of legendEl. I'm using the same type as in the master branch and I'm having this issue... Which means it's not the type that's the problem.
It's happening in other locations too:
    src/Lines.ts(348,40): error TS2769: No overload matches this call.
      Overload 1 of 2, '(data: [number, number][]): string', gave the following error.
        Argument of type '[number[]]' is not assignable to parameter of type '[number, number][]'.
      Overload 2 of 2, '(data: [number, number][]): void', gave the following error.
        Argument of type '[number[]]' is not assignable to parameter of type '[number, number][]'.

Any clue?

So, there's one problem here.
I can't use the previous type of area: d3.Line<[number, number]>; for  reasons that I can't understand :/
If I use it instead of the current any that I'm using I get the following error:
src/Lines.ts(433,23): error TS2339: Property 'y0' does not exist on type 'Line<[number, number]>'.
And a few others.

One more question, what are these empty methods down here?
https://github.com/bqplot/bqplot/blob/5595c241093d6f300eac76795d03217c625922bc/js/src/Lines.ts#L663-L670

One more question, what are these empty methods down here?

Those are implementations of the parent's abstract methods. We need them although they don't do anything.
But the fact they don't do anything is a good clue showing that Lines is not implemented correctly, it should implement those methods if we want lines to behave the same as other marks when it comes to selecting/styling marks.

Is bar.ts a good place to look for a similar implementation?
I can do this in this PR.

Is bar.ts a good place to look for a similar implementation? I can do this in this PR.

Maybe Scatter/ScatterBase is the best place to look at, it's the only mark that is fully implemented.
I think it would make sense to have it in a separate PR :)

I added a few other tests for the model.set since I was there already.

Ok... I knew this file would be problem cuz I got confused with the type stuff, I have to go from the place that I'm in now... Will prbb do it later tonight or tomorrow morning!

@martinRenou I think it's finally ready?

Hey @martinRenou, so this is a hard to catch bug. A few things I learned:

The problem lies on setting the color of the mark. The mark you're using doesn't matter, the color attr. is not being persisted and it's defaulted to black.
The image we get out of galata is this weird one:

but in fact if you run the notebook you'll have the following:

Which is much more similar to the one expected:

So, I think galata is just showing us what is the difference between them?

This bug didn't happen before, it was after you squashed things... I was wondering if you remember changing something in this regard @martinRenou?
I went through the diff a couple of times...
This part is different:
@@ -543,6 +543,24 @@ export abstract class Mark extends widgets.WidgetView {
     return _.compact(hit_check).length > 0;
   }

+  // For backward-compatibility with bqplot plugins
+  // TODO Remove it when we make a backward-incompatible release
+  set x_padding(value: number) {
+    this.xPadding = value;
+  }
+
+  get x_padding(): number {
+    return this.xPadding;
+  }
+
+  set y_padding(value: number) {
+    this.yPadding = value;
+  }
+
+  get y_padding(): number {
+    return this.yPadding;
+  }
+

But doesn't really make a diff. I think. It's literally just changing variable names from snake to camel case.

Really cool this galata thing. Thank you for find the issue Martin! <3

Really cool this galata thing

Yep! We really should add more of those tests and use Galata in more projects :D It's super useful already


Client Allocation Request for: Etile

Client Allocation Questions
Core Information

Name: Etile
Website / Social Media: https://keybase.io/Etile0
Region: Europe
DataCap Requested: 20TiB
Addresses to be Notarized: f1shjlc4hfvgvo2a25htew5g4ow2la6hlyei663vy
Notary Requested: cryptowhizzard

@Etile0 Please subscribe to notifications for this Issue to be aware of updates. Notaries may request additional information on the Issue.

Hi There,
Thanks for your application!
Can you tell me what data you want to store on filecoin? What is your relationship with Keybase?

Hello!
Keybase is just a secure messaging platform I use, it's not a website I own.
I would like to use filecoin to store some encrypted personal data (backups and such) and probably some videos and/or music me or my friends make which we want to archive (most of that is probably going to be unencrypted)
Thank you for your reply!


[xenial] Support for multipath and open-iscsi

We would like to ease the effort required to integrate CFCR with iscsi storage.
An operation team I am working with is hoping to use Pure Storage with iscsi through vSphere datastores. To enable this, we need the following packages :

multipath-tools_0.5.0+git1.656f8865-5ubuntu2_amd64.deb
open-iscsi_2.0.873+git0.3b4b4500-14ubuntu3_amd64.deb

I would be happy to provide a PR but wanted to discuss with you if that should be contributed in the upstream stemcell or in the CFCR bosh release.

Do you still need these packages? If so, please re-open.

A release's dependencies should be packaged in the release where possible. Doing so provides several benefits:

More flexibility in how the dependency is configured
Less overhead - stemcells have security compliance requirements that must be met, releases do not.
Makes the release more portable; don't have to be on a version of the stemcell that has been updated to have the dependency
Reduces the likelihood of conflicting with other releases because the packages are only installed on the VMs that need them, rather than every VM.


Service not saving the record to the folder.

hi @nickpourazima .
I've another error right now, when trying to run the service...
Everything is working fine, but the "record" it's not saved to the folder. And this is the error trace..
sudo systemctl status audioGuestBook.service
● audioGuestBook.service - Rotary Phone Guest Book Project
     Loaded: loaded (/etc/systemd/system/audioGuestBook.service; enabled; vendor preset: enabled)
     Active: active (running) since Sun 2023-08-20 19:23:23 BST; 5min ago
   Main PID: 7621 (python3)
      Tasks: 6 (limit: 1595)
        CPU: 4.266s
     CGroup: /system.slice/audioGuestBook.service
             └─7621 python3 /home/isaacmassot/Desktop/rotaryGuestBook/audioGuestBook.py

Aug 20 19:23:36 raspberrypi env[7621]:     super(HoldMixin, self)._fire_activated()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/mixins.py", line 364, in _fire_activated
Aug 20 19:23:36 raspberrypi env[7621]:     self.when_activated()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/Desktop/rotaryGuestBook/audioGuestBook.py", line 52, in off_hook
Aug 20 19:23:36 raspberrypi env[7621]:     audio_interface.stop()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/Desktop/rotaryGuestBook/audioInterface.py", line 65, in stop
Aug 20 19:23:36 raspberrypi env[7621]:     self.stream.stop_stream()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/.local/lib/python3.9/site-packages/pyaudio.py", line 528, in stop_stream
Aug 20 19:23:36 raspberrypi env[7621]:     pa.stop_stream(self._stream)
Aug 20 19:23:36 raspberrypi env[7621]: OSError: Stream not open

if I chek with journalctl -xe
ug 20 19:23:29 raspberrypi env[7621]: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
Aug 20 19:23:29 raspberrypi env[7621]: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pcm_oss.c:377:(_snd_pcm_oss_open) Unknown field port
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pcm_oss.c:377:(_snd_pcm_oss_open) Unknown field port
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pulse.c:242:(pulse_connect) PulseAudio: Unable to connect: Connection refused
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pulse.c:242:(pulse_connect) PulseAudio: Unable to connect: Connection refused
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pcm_a52.c:823:(_snd_pcm_a52_open) a52 is only for playback
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pcm_usb_stream.c:486:(_snd_pcm_usb_stream_open) Invalid type for card
Aug 20 19:23:29 raspberrypi env[7621]: ALSA lib pcm_usb_stream.c:486:(_snd_pcm_usb_stream_open) Invalid type for card
Aug 20 19:23:30 raspberrypi env[7621]: Cannot connect to server socket err = No such file or directory
Aug 20 19:23:30 raspberrypi env[7621]: Cannot connect to server request channel
Aug 20 19:23:30 raspberrypi env[7621]: jack server is not running or cannot be started
Aug 20 19:23:30 raspberrypi env[7621]: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
Aug 20 19:23:30 raspberrypi env[7621]: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
Aug 20 19:23:36 raspberrypi env[7621]: Traceback (most recent call last):
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/pins/rpigpio.py", line 223, in _call_when_changed
Aug 20 19:23:36 raspberrypi env[7621]:     super(RPiGPIOPin, self)._call_when_changed()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/pins/local.py", line 128, in _call_when_changed
Aug 20 19:23:36 raspberrypi env[7621]:     super(LocalPiPin, self)._call_when_changed(
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/pins/pi.py", line 298, in _call_when_changed
Aug 20 19:23:36 raspberrypi env[7621]:     method(ticks, state)
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/input_devices.py", line 182, in _pin_changed
Aug 20 19:23:36 raspberrypi env[7621]:     self._fire_events(ticks, bool(self._state_to_value(state)))
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/mixins.py", line 401, in _fire_events
Aug 20 19:23:36 raspberrypi env[7621]:     self._fire_activated()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/mixins.py", line 447, in _fire_activated
Aug 20 19:23:36 raspberrypi env[7621]:     super(HoldMixin, self)._fire_activated()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/usr/lib/python3/dist-packages/gpiozero/mixins.py", line 364, in _fire_activated
Aug 20 19:23:36 raspberrypi env[7621]:     self.when_activated()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/Desktop/rotaryGuestBook/audioGuestBook.py", line 52, in off_hook
Aug 20 19:23:36 raspberrypi env[7621]:     audio_interface.stop()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/Desktop/rotaryGuestBook/audioInterface.py", line 65, in stop
Aug 20 19:23:36 raspberrypi env[7621]:     self.stream.stop_stream()
Aug 20 19:23:36 raspberrypi env[7621]:   File "/home/isaacmassot/.local/lib/python3.9/site-packages/pyaudio.py", line 528, in stop_stream
Aug 20 19:23:36 raspberrypi env[7621]:     pa.stop_stream(self._stream)
Aug 20 19:23:36 raspberrypi env[7621]: OSError: Stream not open
Aug 20 19:23:42 raspberrypi sudo[7676]: isaacmassot : TTY=pts/0 ; PWD=/home/isaacmassot/Desktop/rotaryGuestBook ; USER=root ; COMMAND=/usr/bin/systemctl status audioGue>
Aug 20 19:23:42 raspberrypi sudo[7676]: pam_unix(sudo:session): session opened for user root(uid=0) by isaacmassot(uid=1000)
Aug 20 19:23:42 raspberrypi sudo[7676]: pam_unix(sudo:session): session closed for user root
Aug 20 19:27:46 raspberrypi sudo[8202]: isaacmassot : TTY=pts/0 ; PWD=/home/isaacmassot/Desktop/rotaryGuestBook ; USER=root ; COMMAND=/usr/bin/systemctl status audioGue>
Aug 20 19:27:46 raspberrypi sudo[8202]: pam_unix(sudo:session): session opened for user root(uid=0) by isaacmassot(uid=1000)
Aug 20 19:27:46 raspberrypi sudo[8202]: pam_unix(sudo:session): session closed for user root

Any idea on how to solved? Thanks in advance,

Hi @isaacmg410 , apologies for the late response. We're you able to resolve this? Was it a permissions issue?


Opening an MIB file from HDD is slow with current master

How to reproduce:

LiberTEM current master
Windows 10 (Merlin PC)
Stored on a HDD that can reach about 100 MB/s write speed in a 1:1 copy, as an approximate performance measure.
MIB file 256x256x256x256 with all frames in a single file
D:\Merlin_data\Janghyun\20200619 - SNBD Test\04_256X256_1ms_TV160/Test.hdr
File is not in the FS cache
Open with LiberTEM GUI and add basic analysis

What happens:

Read performance 7-8 MB/s, unusably slow

Expected: Reaches at least the read performance of LiberTEM 0.5.0, about 70 MB/s
If the file is in the FS cache, read performance is normal (good).
Possibly related to #814?

If the file is in the FS cache, read performance is normal (good).

This points to the seek patterns for tiled reading being the problem - apparently Windows doesn't recognize those as "linear" and doesn't perform any read-ahead. I think we are at a point where we should make a general decision how to handle this - if we want to have usable performance on slow-seek media, we need to read in a completely different way than if we want to have optimal performance on fast SSDs. On an NVMe SSD, we theoretically can increase performance by dispatching even more parallel read commands (using liburing etc.), possibly reading deeper tiles, which would completely tank HDD perf.
I think it may very well be worthwhile to explore this, as other I/O "backends" can have similar issues, like reading from parallel file systems, NFS, or via HTTP. I'll see that I do some local testing soon, and then we should discuss how we prioritize a fix for this issue.

we should make a general decision how to handle this - if we want to have usable performance on slow-seek media

On the one hand, since performance was acceptable before and people including us still use such media, we should at least be on par with 0.5.0. However, we should also keep the massive performance gains of optimized tiled processing.
Having different code paths for slow-seek and fast-seek media could get messy, though. More code volume, and how to decide which one to use? Are there OS interfaces that can tell on what kind of storage a file is kept? Should we let users decide?

By the way, can this be helpful for mmapped data, perhaps?

https://linux.die.net/man/2/fadvise
https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-prefetchvirtualmemory?redirectedfrom=MSDN

In any case, a good use case for the benchmarking infrastructure... #819

On the one hand, since performance was acceptable before and people including us still use such media, we should at least be on par with 0.5.0. However, we should also keep the massive performance gains of optimized tiled processing.

how to decide which one to use

I think we only have a chance of making the user decide this (i.e. opt in to the "slow HDD" I/O backend) - I don't think there is a portable API to find expected performance for block devices. On Linux, this is definitely possible to find out, for example in /sys/block/sda/queue/ on my system there is a file rotational, and there are also I/O statistics available, like average I/O latency ("total wait time for read requests" and "number of read I/Os processed" from /sys/block/*/stat). But automatically deciding could also make performance unpredictable...
Maybe we can get some inspiration from data bases here - they have tuning parameters that specify how costly some operations are, for example random_page_cost on PostgreSQL can be changed to tip the scales between doing a sequential scan over the whole table, vs. doing more "random" reads using indexes. With a HDD it can be faster to "waste" bandwidth and scan the whole table than having to seek repeatedly to find specific values, depending on what percentage of the whole table we expect to hit with the query. Of course, this also depends on the likelyhood of hitting the filesystem cache, ...

https://linux.die.net/man/2/fadvise

Yeah, see here: https://github.com/LiberTEM/LiberTEM/blob/cf241433b094473c9edc38e80c8d79f4a9e8ffac/src/libertem/io/dataset/base/backend.py#L240
Could be made a bit smarter about the values of the roi, but in general it is there.

https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-prefetchvirtualmemory?redirectedfrom=MSDN

Thanks for the link! I'll have a look if there is a good way to use this from Python...

See also discussion here: https://bugs.python.org/issue32941

Yeah, see here:

👍 nice, didn't know it was implemented already!

In any case, a good use case for the benchmarking infrastructure... #819

How would that work, can we do a run of I/O benchmarks on different storage locations on the same host and have them distinguishable in the result history?

can we do a run of I/O benchmarks on different storage locations on the same host and have them distinguishable in the result history?

We can use pytest.mark.parametrize and/or add specific metadata. One could, for example, add the full path or environment variables with a base directory to extra_info. That way one can distinguish separate runs on different locations.

A few thoughts on how to handle this:
The optimal IO strategy depends on seek time, optimal block size, transfer rate and the ability to handle concurrent requests efficiently. In reality we may have the following sources when reading: File system cache, SSD, HDD, and network

File system cache: No seek time, block size is single cache line, highest possible transfer rate, handles concurrent requests nearly instantaneous.

Optimal strategy: Memory mapping, direct access of exactly the data that is needed when it is needed.

SSD: Very low seek time independent of jump size, optimal block size in the kB to MB range(?), transfer rate high, speed increase from concurrent requests.

Optimal strategy: Tiled reading (concurrent read of several blocks with 10s to 100s of kB each) should work well. Direct IO or Liburing for files larger than RAM. Memory mapping and reading exactly what is needed when it is needed should work reasonably well. fadvise() in combination with memory mapping should help a bit if files fit into RAM to avoid overheads from page faults.

HDD: Large seek time, block size 4k (?), moderate transfer rate, sequential reads are highly favored, doesn't handle concurrency well at all.

Optimal strategy: Sequential read of contiguous portions. Bridging a few "useless" MB vs. discontinuous reads probably favored (seek time vs. transfer rate), further tiling / disassembly from memory buffer. Memory mapping only if fadvise() or the Windows equivalent work reliably to let the OS prefetch enough data into the cache with sequential reads so that our memory access patterns don't trigger page faults.

Network: Moderate to large latency (software stack latency + network latency + latency of storage), small to medium transfer rate, may or may not handle concurrent access well, may have overheads when accessing small blocks -- we don't know what is on the other side!

Optimal strategy: Avoid page faults when memory mapping (round trip time), i.e. use only if fadvise() can avoid them reliably. Probably easiest to read larger contiguous blocks at a time similar to HDD. Saving bandwidth may, in tendency, be more favorable compared to HDD.

The way I can see it, the decision tree boils down to the following items:

Is the file in the file system cache? If yes, mmap() wins by a margin.
Is the file on an SSD? If yes, mmap() will do reasonably well under all circumstances.
Otherwise, reading larger blocks into memory will do better. SSDs will work well,, too.

Reading large blocks will do reasonably well under all scenarios, while reading small portions with mmap() can have dramatic negative effects on HDD and possibly network, as illustrated by this Issue. That means the large block strategy can be a good failsafe.
Since it would be hard to find out what the current situation is beforehand and it changes dynamically (in the FS cache vs. outside, other processes doing things), we could implement a simple heuristic that switches between mmap() and reading large blocks: We simply monitor the effective read rate for each tile, and use whatever works better.
As an example, we could process three tiles with each mmap() and large contiguous read(). The first ones are warm-up, and we just use what gave the best average result for the remaining two. Furthermore, we can monitor the read rate. If it falls below, say, 50 MB/s per worker, we switch to the large block strategy. That one will always be a safe bet, and at 50 MB/s the additional memory transfer will have hardly any effect.
Possibly we can even keep it simpler: First tile mmap() warmup (Numba compilation etcetcetc). Then continue with mmap() and monitor. If below 50 MB/s, switch to large blocks for rest of partition.
Possibly we can also let the user override the read strategy as desired, in case the user is competent and the heuristic is not optimal. That's similar to the "direct IO" option we currently have. Options are mmap(), large blocks, DIO, and possibly liburing in the future (refs #268).

For testing, we could have paths for the same data on different storage locations determined by, for example environment variables or a sort of "host file". That way we can run benchmarks with pytest.mark.parametrize() on all the storage locations in one run with the same tests and skip the ones where we don't have a file on that system.
Proposal for a reasonable order:

Make it reproducible and testable, for example using Moellenstedt
See if we can get the "Windows fadvise()" going and if it already resolves the issue.

If it doesn't:

Implement "large contiguous read" strategy at least for MIB, confirm effective remedy of this issue for HDD.
Implement user control over read strategy, which will facilitate further benchmarking
Implement "large contiguous read" strategy for other dataset types, if not already done in 2
Implement rate monitoring and dynamic switching of read strategy

Perhaps I can already start to get benchmarks going on different storage locations, at the same time switching over to our new data management directory structure?

As mentioned in  #887, this appears to be a Windows-specific problem.
Can you try to run the benchmarks with fadvise disabled? Just adding a return statement here should do the trick.
We should see if not using fadvise on Linux means we get the same problem, and then decide if we want to implement a Windows-specific fadvise replacement, or if we want to implement buffered reading and enable it by default for Windows only.

 The test file is smaller and split up in small MIB files, while the issue was caused by a larger monolithic MIB file.

In any case, the results with cold cache and fadvise() were all a bit better than the ones without fadvise(), so it DOES help some.

Thanks for testing this! It would be good to track these numbers over time.

Next step would be reproducing on Windows, and if this doesn't reproduce the issue then test with the exact same file that created the issue. The test file is smaller and split up in small MIB files, while the issue was caused by a larger monolithic MIB file.

Let me know if you need any help with that. We can also create synthetic test data if needed (it's easy to create a large MIB file from many small ones, for example).

Benchmark results with Windows:

--------------------------------------------------------------------------------------------------------- benchmark: 6 tests --------------------------------------------------------------------------------------------------------
Name (time in s)                                                           Min                Max               Mean            StdDev             Median               IQR            Outliers     OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_mask[C:/Users/dwebe/Desktop/reference-cold_cache]                 62.7528 (91.98)    62.7528 (91.98)    62.7528 (91.98)    0.0000 (1.0)      62.7528 (91.98)    0.0000 (1.0)           0;0  0.0159 (0.01)          1           1
test_mask[C:/Users/dwebe/Desktop/reference-warm_cache]                  0.8421 (1.23)      0.8421 (1.23)      0.8421 (1.23)     0.0000 (1.0)       0.8421 (1.23)     0.0000 (1.0)           0;0  1.1876 (0.81)          1           1
test_mask_firstrun[C:/Users/dwebe/Desktop/reference-cold_executor]     13.0089 (19.07)    13.0089 (19.07)    13.0089 (19.07)    0.0000 (1.0)      13.0089 (19.07)    0.0000 (1.0)           0;0  0.0769 (0.05)          1           1
test_mask_firstrun[C:/Users/dwebe/Desktop/reference-warm_executor]      0.8810 (1.29)      0.8810 (1.29)      0.8810 (1.29)     0.0000 (1.0)       0.8810 (1.29)     0.0000 (1.0)           0;0  1.1351 (0.77)          1           1
test_sequential[C:/Users/dwebe/Desktop/reference-cold_cache]            8.7860 (12.88)     8.7860 (12.88)     8.7860 (12.88)    0.0000 (1.0)       8.7860 (12.88)    0.0000 (1.0)           0;0  0.1138 (0.08)          1           1
test_sequential[C:/Users/dwebe/Desktop/reference-warm_cache]            0.6823 (1.0)       0.6823 (1.0)       0.6823 (1.0)      0.0000 (1.0)       0.6823 (1.0)      0.0000 (1.0)           0;0  1.4657 (1.0)           1           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

The issue is clearly reproduced with this dataset and benchmark on Windows. The difference between Linux and Windows is striking.

Thanks for testing this! It would be good to track these numbers over time.

Yes, pytest-benchmarks can do that and I believe we already have a few "dummy" runs. We postponed regular tests until we have more benchmarks. Now might be a good time to get started with that. :-) Perhaps we can discuss a strategy how to organize results and make them comparable?

For posterity, this code should prefetch memeory, but doesn't improve the benchmark results or triggers disk activity if the cache is empty. Not sure if I called it right, though.
class MEMORY_RANGE_ENTRY(ctypes.Structure):
    _fields_ = [
        ("VirtualAddress", ctypes.c_void_p),
        ("NumberOfBytes", ctypes.c_size_t),
    ]

def prefetch_mmap(memmap):
    first = ctypes.c_char_p(memmap[:1])
    mentry = MEMORY_RANGE_ENTRY(
        VirtualAddress=ctypes.cast(first, ctypes.c_void_p),
        NumberOfBytes=memmap.size(),
    )
    result = ctypes.windll.kernel32.PrefetchVirtualMemory(
        ctypes.windll.kernel32.GetCurrentProcess(),
        ctypes.c_ulong(1),
        mentry,  # or perhaps ctypes.c_void_p(ctypes.addressof(mentry))
        ctypes.c_ulong(0)
    )
    if result == 0:
        raise RuntimeError("Return code 0 means error, see https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-prefetchvirtualmemory")

Since the issue is reproduced on Windows and prefetching didn't work, we'll have to implement the "large block" reading strategy, it seems.
Linux seems to do an excellent job to translate the parallel tiled memory access pattern into larger contiguous disk reads, while Windows produces a flurry of seeks.

As an alternative to changing the reading strategy in this release, perhaps one could change the tiling scheme instead to use the same scheme as 0.5 if IO performance is slow? That could be done per partition. Possibly the UDFRunner could raise an "IOTooSlowExeption" when it detects slow IO after processing a few tiles,  and start over with tiling that will give sequential access in run_for_partition()?
The real sequential reading strategy could then be implemented in #473.

As a follow-up comment, the current buffered reader in 0.6.0rc2 reaches about 30-40 % of the theoretical read performance of the HDD in the "mobile workstation" machine with the 4 GB EMPAD dataset. IMO that's good enough for now, one could just see if #473 can perhaps beat that at some point. :-)
The performance impact of buffered reading vs memmap for files in the cache is quite noticeable on that machine, by the way. The CPU in that machine has very poor memory bandwidth. It seems that the additional copy operation has a big impact on performance for that reason.


Dead interpolation elimination improvements

The optimizer doesn't understand the effects of appendLiteral(_:) and appendInterpolation(_:) calls, which only modify self. Because of this, it can't tell that an interpolation can be eliminated if the resulting string is unused, which causes a (no-assertions-only) test failure and some code size regressions.
This is an attempt to help DeadCodeElimination along. The first revision benchmarked poorly when I tried it a few days ago; I'm just pushing it now to get a baseline, and then I'll experiment a little more from there. Currently pretty hacky, too.
This could be a dead end. If it isn't, it should resolve SR-9008, rdar://45294605, and rdar://45788348.

Build comment file:
Performance: -O

TEST
OLD
NEW
DELTA
RATIO

Improvement

IterateData
1790
1617
-9.7%
1.11x

Code size: -O

TEST
OLD
NEW
DELTA
RATIO

Improvement

DeadArray.o
1940
1596
-17.7%
1.22x

Performance: -Osize

TEST
OLD
NEW
DELTA
RATIO

Improvement

DictionaryKeysContainsNative
36
29
-19.4%
1.24x

IterateData
1842
1643
-10.8%
1.12x (?)

CStringLongAscii
3528
3285
-6.9%
1.07x

Code size: -Osize

TEST
OLD
NEW
DELTA
RATIO

Improvement

DeadArray.o
1906
1578
-17.2%
1.21x

Performance: -Onone

TEST
OLD
NEW
DELTA
RATIO

Improvement

Histogram
6711
5654
-15.8%
1.19x

CStringLongAscii
3662
3373
-7.9%
1.09x

QueueGeneric
20417
19034
-6.8%
1.07x

Code size: Swift libraries

TEST
OLD
NEW
DELTA
RATIO

Regression

libswiftFoundation.dylib
1683456
1708032
+1.5%
0.99x

Improvement

libswiftSwiftReflectionTest.dylib
53248
45056
-15.4%
1.18x

How to read the data
The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.
If you see any unexpected regressions, you should consider fixing the regressions before you merge the PR.
Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

Wow, much better than expected. Let's do a longer run.

@swift-ci please benchmark

@swift-ci please test

@swift-ci please benchmark

!!!

IterateData
1515
1356
-10.5%
1.12x

Code size: -Osize

TEST
OLD
NEW
DELTA
RATIO

Regression

DictionaryKeysContains.o
11459
12867
+12.3%
0.89x

WordCount.o
41708
43244
+3.7%
0.96x

SequenceAlgos.o
23148
23516
+1.6%
0.98x

TwoSum.o
5277
5341
+1.2%
0.99x

Improvement

DeadArray.o
1842
1458
-20.8%
1.26x

ByteSwap.o
1634
1614
-1.2%
1.01x

Performance: -Onone

TEST
OLD
NEW
DELTA
RATIO

Regression

MapReduceLazyCollection
21239
25803
+21.5%
0.82x

MapReduceLazyCollectionShort
31806
38388
+20.7%
0.83x

Code size: -swiftlibs

TEST
OLD
NEW
DELTA
RATIO

Regression

libswiftFoundation.dylib
1593344
1609728
+1.0%
0.99x

Improvement

libswiftSwiftReflectionTest.dylib
49152
45056
-8.3%
1.09x

How to read the data
The tables contain differences in performance which are larger than 8% and
differences in code size which are larger than 1%.
If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.
Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

--------------

I'm reasonably satisfied with these results. Comments on some surprises:

Smaller-than-expected libswiftSwiftReflectionTest.dylib improvement: The baseline changed. On Sunday, master was building a 53k dylib; on Thursday, it built a 49k dylib. In both runs, this PR built a 45k version.
DictionaryKeysContains.o size regression: The inliner seems to make different decisions about Dictionary.init<A>(uniqueKeysWithValues:)—in the branch, it doesn't inline _NativeDictionary.merge<A>(_:isUnique:uniquingKeysWith:) but instead keeps it as a separate specialized function. There's a string interpolation in the merge method which generates a fatalError() message, and it looks like it's not inlining calls to DefaultStringInterpolation.init, so perhaps that's changing the inlining cost enough so that it makes a different decision? (In any case, I noticed some slightly questionable code in some of these members, so it might be changing soon...)

The WordCount.o, SequenceAlgos.o, and TwoSum.o tests are heavy on calls to CheckResults(_:), a function with a string interpolation in its (never-taken) failure branch.
If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.
Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

--------------

@swift-ci please test

Build failed
Swift Test OS X Platform
Git Sha - 32f043da5c12a520e8c570ed059dc52b6aa6c599

That's the CI's usual "bad hash" failure. Tests are running here.

Build failed
Swift Test Linux Platform
Git Sha - 32f043da5c12a520e8c570ed059dc52b6aa6c599

DictionaryKeysContains.o

I think we can live with the code size regression for now.

@swift-ci please benchmark

Build comment file:
Code size: -O

TEST
OLD
NEW
DELTA
RATIO

Regression

DictionaryKeysContains.o
11767
13431
+14.1%
0.88x

WordCount.o
44228
45732
+3.4%
0.97x

Improvement

DeadArray.o
1892
1532
-19.0%
1.23x

Performance: -Osize

TEST
OLD
NEW
DELTA
RATIO

Regression

CharacterLiteralsLarge
100
111
+11.0%
0.90x

Code size: -Osize

TEST
OLD
NEW
DELTA
RATIO

Regression

DictionaryKeysContains.o
11455
12895
+12.6%
0.89x

WordCount.o
40516
41924
+3.5%
0.97x

Improvement

DeadArray.o
1890
1506
-20.3%
1.25x

DictTest4.o
20855
20631
-1.1%
1.01x

Performance: -Onone

TEST
OLD
NEW
DELTA
RATIO

Regression

ArrayOfPOD
777
858
+10.4%
0.91x (?)

Improvement

ExclusivityIndependent
82
70
-14.6%
1.17x

Code size: -swiftlibs

TEST
OLD
NEW
DELTA
RATIO

Regression

libswiftFoundation.dylib
1519616
1536000
+1.1%
0.99x

Improvement

libswiftSwiftReflectionTest.dylib
40960
32768
-20.0%
1.25x

How to read the data
The tables contain differences in performance which are larger than 8% and
differences in code size which are larger than 1%.
If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.
Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

--------------

What CI meant to say was:
Summary for master full
Unexpected test results, excluded stats for Tagged, NonEmpty, ProcedureKitCloud, GRDB, ModelAssistant, Wordy
Regressions found (see below)

Debug-batch

debug-batch brief
debug-batch detailed

Release

release brief
release detailed

Debug-batch

debug-batch brief

Regressed (1)

name
old
new
delta
delta_pct

Frontend.NumInstructionsExecuted
21,299,318,627,006
23,930,268,593,075
2,630,949,966,069
12.35% :no_entry:

Improved (0)

name
old
new
delta
delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name
old
new
delta
delta_pct

LLVM.NumLLVMBytesOutput
911,949,140
911,727,304
-221,836
-0.02%

time.swift-driver.wall
2246.9s
2256.4s
9.5s
0.42%

debug-batch detailed

Regressed (1)

name
old
new
delta
delta_pct

Frontend.NumInstructionsExecuted
21,299,318,627,006
23,930,268,593,075
2,630,949,966,069
12.35% :no_entry:

Improved (2)

name
old
new
delta
delta_pct

Driver.NumDriverPipePolls
304,783
292,005
-12,778
-4.19% :white_check_mark:

Driver.NumDriverPipeReads
345,376
331,797
-13,579
-3.93% :white_check_mark:

Unchanged (delta < 1.0% or delta < 100.0ms) (92)

name
old
new
delta
delta_pct

AST.NumASTBytesAllocated
45,359,895,288
45,451,505,008
91,609,720
0.2%

AST.NumDecls
65,849
65,849
0
0.0%

AST.NumDependencies
152,683
152,689
6
0.0%

AST.NumImportedExternalDefinitions
960,730
960,730
0
0.0%

AST.NumInfixOperators
24,640
24,640
0
0.0%

AST.NumLinkLibraries
0
0
0
0.0%

AST.NumLoadedModules
180,845
180,845
0
0.0%

AST.NumLocalTypeDecls
113
113
0
0.0%

AST.NumObjCMethods
12,528
12,528
0
0.0%

AST.NumPostfixOperators
13
13
0
0.0%

AST.NumPrecedenceGroups
12,687
12,687
0
0.0%

AST.NumPrefixOperators
71
71
0
0.0%

AST.NumReferencedDynamicNames
101
101
0
0.0%

AST.NumReferencedMemberNames
3,046,567
3,046,567
0
0.0%

AST.NumReferencedTopLevelNames
210,981
210,981
0
0.0%

AST.NumSourceBuffers
288,503
288,503
0
0.0%

AST.NumSourceLines
2,107,547
2,107,547
0
0.0%

AST.NumSourceLinesPerSecond
1,167,653
1,160,805
-6,848
-0.59%

AST.NumTotalClangImportedEntities
3,562,456
3,565,535
3,079
0.09%

AST.NumUsedConformances
185,274
185,274
0
0.0%

Driver.ChildrenMaxRSS
68,565,780,480
68,615,297,024
49,516,544
0.07%

Driver.DriverDepCascadingDynamic
0
0
0
0.0%

Driver.DriverDepCascadingExternal
0
0
0
0.0%

Driver.DriverDepCascadingMember
0
0
0
0.0%

Driver.DriverDepCascadingNominal
0
0
0
0.0%

Driver.DriverDepCascadingTopLevel
0
0
0
0.0%

Driver.DriverDepDynamic
0
0
0
0.0%

Driver.DriverDepExternal
0
0
0
0.0%

Driver.DriverDepMember
0
0
0
0.0%

Driver.DriverDepNominal
0
0
0
0.0%

Driver.DriverDepTopLevel
0
0
0
0.0%

Driver.NumDriverJobsRun
13,529
13,529
0
0.0%

Driver.NumDriverJobsSkipped
0
0
0
0.0%

Driver.NumProcessFailures
0
0
0
0.0%

Frontend.MaxMallocUsage
352,809,515,144
353,114,816,472
305,301,328
0.09%

Frontend.NumProcessFailures
0
0
0
0.0%

IRModule.NumIRAliases
92,680
92,680
0
0.0%

IRModule.NumIRBasicBlocks
3,587,318
3,587,289
-29
-0.0%

IRModule.NumIRComdatSymbols
0
0
0
0.0%

IRModule.NumIRFunctions
1,632,326
1,632,326
0
0.0%

IRModule.NumIRGlobals
1,857,354
1,857,354
0
0.0%

IRModule.NumIRIFuncs
0
0
0
0.0%

IRModule.NumIRInsts
42,089,475
42,077,011
-12,464
-0.03%

IRModule.NumIRNamedMetaData
66,177
66,177
0
0.0%

IRModule.NumIRValueSymbols
3,120,194
3,120,194
0
0.0%

LLVM.NumLLVMBytesOutput
911,949,140
911,727,304
-221,836
-0.02%

Parse.NumFunctionsParsed
2,170,339
2,170,339
0
0.0%

Parse.NumIterableDeclContextParsed
865,730
865,730
0
0.0%

SILModule.NumSILGenDefaultWitnessTables
0
0
0
0.0%

SILModule.NumSILGenFunctions
1,286,682
1,286,682
0
0.0%

SILModule.NumSILGenGlobalVariables
26,821
26,821
0
0.0%

SILModule.NumSILGenVtables
10,171
10,171
0
0.0%

SILModule.NumSILGenWitnessTables
37,360
37,360
0
0.0%

SILModule.NumSILOptDefaultWitnessTables
0
0
0
0.0%

SILModule.NumSILOptFunctions
1,170,065
1,171,362
1,297
0.11%

SILModule.NumSILOptGlobalVariables
27,550
27,550
0
0.0%

SILModule.NumSILOptVtables
16,355
16,355
0
0.0%

SILModule.NumSILOptWitnessTables
72,507
72,507
0
0.0%

Sema.AccessLevelRequest
1,912,680
1,913,194
514
0.03%

Sema.DefaultAndMaxAccessLevelRequest
45,840
45,843
3
0.01%

Sema.EnumRawTypeRequest
13,205
13,205
0
0.0%

Sema.ExtendedNominalRequest
2,675,157
2,675,431
274
0.01%

Sema.InheritedDeclsReferencedRequest
84,613,324
84,668,705
55,381
0.07%

Sema.InheritedTypeRequest
441,224
441,287
63
0.01%

Sema.IsDynamicRequest
1,535,733
1,535,733
0
0.0%

Sema.IsObjCRequest
1,333,835
1,333,978
143
0.01%

Sema.NamedLazyMemberLoadFailureCount
18,161
18,184
23
0.13%

Sema.NamedLazyMemberLoadSuccessCount
13,018,294
13,012,199
-6,095
-0.05%

Sema.NominalTypeLookupDirectCount
24,981,204
24,991,188
9,984
0.04%

Sema.NumConformancesDeserialized
3,879,389
3,882,556
3,167
0.08%

Sema.NumConstraintScopes
13,045,831
13,046,856
1,025
0.01%

Sema.NumConstraintsConsideredForEdgeContraction
29,316,344
29,316,587
243
0.0%

Sema.NumDeclsDeserialized
30,827,826
30,873,244
45,418
0.15%

Sema.NumDeclsValidated
1,645,646
1,645,641
-5
-0.0%

Sema.NumFunctionsTypechecked
920,531
920,531
0
0.0%

Sema.NumGenericSignatureBuilders
872,637
872,284
-353
-0.04%

Sema.NumLazyGenericEnvironments
6,157,722
6,157,904
182
0.0%

Sema.NumLazyGenericEnvironmentsLoaded
165,400
165,424
24
0.01%

Sema.NumLazyIterableDeclContexts
4,932,562
4,933,380
818
0.02%

Sema.NumLeafScopes
8,944,710
8,945,570
860
0.01%

Sema.NumTypesDeserialized
10,882,131
10,888,374
6,243
0.06%

Sema.NumTypesValidated
1,078,137
1,078,132
-5
-0.0%

Sema.NumUnloadedLazyIterableDeclContexts
3,500,966
3,499,675
-1,291
-0.04%

Sema.OverriddenDeclsRequest
4,103,315
4,121,991
18,676
0.46%

Sema.RequirementRequest
54,623
54,619
-4
-0.01%

Sema.SelfBoundsFromWhereClauseRequest
50,408,559
50,467,490
58,931
0.12%

Sema.SetterAccessLevelRequest
108,626
108,626
0
0.0%

Sema.SuperclassDeclRequest
66,573,449
66,597,424
23,975
0.04%

Sema.SuperclassTypeRequest
30,277
30,277
0
0.0%

Sema.TypeDeclsFromWhereClauseRequest
26,254
26,257
3
0.01%

Sema.USRGenerationRequest
6,141,318
6,181,319
40,001
0.65%

Sema.UnderlyingTypeDeclsReferencedRequest
2,296,019
2,296,093
74
0.0%

Release

release brief

Regressed (0)

name
old
new
delta
delta_pct

Improved (0)

name
old
new
delta
delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (3)

name
old
new
delta
delta_pct

Frontend.NumInstructionsExecuted
24,042,045,391,202
24,042,730,935,196
685,543,994
0.0%

LLVM.NumLLVMBytesOutput
811,141,394
811,010,570
-130,824
-0.02%

time.swift-driver.wall
4466.2s
4465.0s
-1.2s
-0.03%

release detailed

Regressed (0)

name
old
new
delta
delta_pct

Improved (0)

name
old
new
delta
delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name
old
new
delta
delta_pct

AST.NumImportedExternalDefinitions
178,485
178,485
0
0.0%

AST.NumLoadedModules
11,404
11,404
0
0.0%

AST.NumTotalClangImportedEntities
602,601
602,601
0
0.0%

AST.NumUsedConformances
186,004
186,004
0
0.0%

IRModule.NumIRBasicBlocks
2,985,101
2,985,023
-78
-0.0%

IRModule.NumIRFunctions
1,338,631
1,338,723
92
0.01%

IRModule.NumIRGlobals
1,465,764
1,465,655
-109
-0.01%

IRModule.NumIRInsts
28,671,707
28,672,463
756
0.0%

IRModule.NumIRValueSymbols
2,620,363
2,620,358
-5
-0.0%

LLVM.NumLLVMBytesOutput
811,141,394
811,010,570
-130,824
-0.02%

SILModule.NumSILGenFunctions
561,090
561,090
0
0.0%

SILModule.NumSILOptFunctions
695,820
696,113
293
0.04%

Sema.NumConformancesDeserialized
1,574,881
1,572,972
-1,909
-0.12%

Sema.NumConstraintScopes
11,500,306
11,500,306
0
0.0%

Sema.NumDeclsDeserialized
3,928,331
3,928,720
389
0.01%

Sema.NumDeclsValidated
855,460
855,460
0
0.0%

Sema.NumFunctionsTypechecked
450,388
450,388
0
0.0%

Sema.NumGenericSignatureBuilders
144,163
144,163
0
0.0%

Sema.NumLazyGenericEnvironments
792,879
792,879
0
0.0%

Sema.NumLazyGenericEnvironmentsLoaded
15,999
15,999
0
0.0%

Sema.NumLazyIterableDeclContexts
531,886
531,906
20
0.0%

Sema.NumTypesDeserialized
2,136,447
2,139,197
2,750
0.13%

Sema.NumTypesValidated
401,820
401,820
0
0.0%

Also, ModelAssistant UPASSed. So, that happened.

So, this passes all tests and doesn't seem to cause any problems as long as NONTRIVIAL_ALLOC_STACK_FOR_EVERYONE is not defined. But with it defined, it seems to introduce incorrect releases in some tests, which makes me worried there's an underlying issue. Reduction for one of these (pop it into test/stdlib and run it with lit; make sure you have a release-mode, no-assertion stdlib):
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

// REQUIRES: objc_interop

// FIXME: rdar://problem/31311598
// UNSUPPORTED: OS=ios
// UNSUPPORTED: OS=tvos
// UNSUPPORTED: OS=watchos

//
// Tests for the NSString APIs as exposed by String
//

import Foundation

_ = "абвг" == "абВГ".lowercased(with: Locale.current)

As far as I can tell, the failing tests all involve bridged NSStrings, and interpolation buffers should ~never be NSStrings. But it's still a concern, and this patch is probably getting too large to land in swift-5.0-branch anyway.


Correct text in opening sentence on Managing Office 365 endpoints

Referring to the first paragraph and sentence on the page there is an extra ""to need" that should be removed. Or alternatively re-worked.
Original text:
Most enterprise organizations that have multiple office locations and a connecting WAN will need to need configuration for Office...
Proposed update:
Most enterprise organizations that have multiple office locations and a connecting WAN will need configuration for Office...

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 8762d73c-32fc-a4f9-162a-b81ae4052ea3
Version Independent ID: 554cc99d-b929-8db0-7ab7-0f666438e8cf
Content: Managing Office 365 endpoints
Content Source: Enterprise/managing-office-365-endpoints.md
Service: o365-administration
GitHub Login: @JoeDavies-MSFT
Microsoft Alias: josephd

@cliffordkennedy  Thank you for your feedback. We have made changes to the document via PR#https://github.com/MicrosoftDocs/OfficeDocs-Enterprise/pull/956
Thanks
Sri

@msbemba Thank you very much for the contribution and updating the documentation with PR. @cliffordkennedy
Hope this update is helpful for you. Thanks for taking out some time to open the issue. Appreciate and encourage you to do the same in future also.


Error overriding $theme-colors Bootstrap 4.1.3

$theme-colors: ( gray-dark: #0000 );

@import "node_modules/bootstrap/scss/bootstrap";

Running the above is producing the following error:
Error in plugin "sass"
Message:
    node_modules\bootstrap\scss\_functions.scss
Error: argument `$color-2` of `mix($color-1, $color-2, $weight: 50%)` must be a color
        on line 85 of node_modules/bootstrap/scss/_functions.scss, in function `mix`
        from line 85 of node_modules/bootstrap/scss/_functions.scss, in function `theme-color-level`
        from line 103 of node_modules/bootstrap/scss/_tables.scss
        from line 17 of node_modules/bootstrap/scss/bootstrap.scss
        from line 16 of wwwroot/scss/custom.scss
>>   @return mix($color-base, $color, $level * $theme-color-interval);

I have also tried adding the functions and variables before changing $theme-colors e.g:
@import "node_modules/bootstrap/scss/functions";
@import "node_modules/bootstrap/scss/variables";

$theme-colors: ( gray-dark: #0000 );

@import "node_modules/bootstrap/scss/bootstrap";

Sorry if I am missing something simple, I don't normally do front-end work. Thanks in advance.

See https://getbootstrap.com/docs/4.1/getting-started/theming/#add-to-map
This should fix it:
$theme-colors: (
  "gray-dark": #000000
);

Now that's removed the error when I compile, but the $theme-colors don't contain the updated color for "gray-dark", it still has the original bootstrap value it gets from $gray-800.

$colors and $theme-colors are two different maps.
See https://github.com/twbs/bootstrap/blob/v4-dev/scss/_variables.scss#L49

Please see the docs on how to customize Sass maps: http://getbootstrap.com/docs/4.1/getting-started/theming/#maps-and-loops


CreateSignatureProvider should support EC-HSM Key type

We have been trying to leverage the JwtSecurityTokenHandler to validate JWT Tokens we're creating. We've recently switched over to using HSM backed elliptic curve keys (EC-HSM) key type from Azure Key Vault and the validateToken method is throwing the following error
'System.NotSupportedException: IDX10621: 'Microsoft.IdentityModel.Tokens.SymmetricSignatureProvider' supports: 'Microsoft.IdentityModel.Tokens.SecurityKey' of types: 'Microsoft.IdentityModel.Tokens.AsymmetricSecurityKey' or 'Microsoft.IdentityModel.Tokens.SymmetricSecurityKey'. SecurityKey received was of type 'Microsoft.IdentityModel.Tokens.JsonWebKey'
Looking through the CreateSignatureProvider method in the CryptoProviderFactory class, the "EC-HSM" key type doesn't appear to be a valid option when iterating through the key types. It seems to only support "EC", "RSA" and "Octet" as valid key type options.
if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.RSA || jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.EllipticCurve)
                            typeofSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
                        else if (jsonWebKey.Kty == JsonWebAlgorithmsKeyTypes.Octet)
                        {
                            typeofSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
                            createAsymmetric = false;
                        }

Since neither of these code blocks get hit, the typeOfSignatureProvider is null and the following code snippet executes further down
if (typeofSignatureProvider == null)
                throw LogHelper.LogExceptionMessage(new NotSupportedException(LogHelper.FormatInvariant(LogMessages.IDX10621, typeof(SymmetricSignatureProvider), typeof(SecurityKey), typeof(AsymmetricSecurityKey), typeof(SymmetricSecurityKey), key.GetType())));

@MoAhuja you can make use of the extensibility of ICryptoProvider
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Tokens/ICryptoProvider.cs
Here is a pointer to test code to give you an idea of how it works.
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/d4d6d3598952410ff96ef8f38c9ca9ce74bca8bf/test/Microsoft.IdentityModel.Tokens.Tests/CryptoExtensibilityTests.cs#L197

This should be done after adding a 4.7.2 target.
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/1507

Here is a link that explains a bit about this key type. https://docs.microsoft.com/en-us/azure/key-vault/keys/about-keys

Here is a link that explains a bit about this key type. https://docs.microsoft.com/en-us/azure/key-vault/keys/about-keys

closing as workaround provided, please reopen if you disagree. thanks!


Cost function: choose between RMSE, NRMSE etc.

Users should be free to choose the cost function.

... and adapt tests to take into account different possible cost functions...


Fixes Jsrt API C99 compatability

ChakraCore.h appears to desire compatibility with C99 however this is currently broken due to the use of nullptr, bool and  C++11 style enums.  Fortunately these problems are easily remedied.

Hi @jcoffland, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@jcoffland, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.
Thanks, MSBOT;

@dotnet-bot test this please

@dotnet-bot test this please

@jcoffland could you please squash the commits ? That should also trigger CI

Ok, I've added the whitespace and squashed the commits.

Any chance this will get merged?  It's a very basic fix and I followed all the guidelines.  Will someone please take responsibility for merging this PR?

@jcoffland CI has been failing on this PR weirdly. Could you please try squash (still seeing 3 commits) and rebase onto latest master and force push your PR? Thanks!

Ok, I believe I've sorted it out.  I had already squashed my commits but then updated my fork by doing a merge which added extra commits to this PR.  I fixed this by redoing the fork update as a rebase.

@dotnet-bot test this please

Did a search and found
JsTTDMoveMode moveMode = (JsTTDMoveMode)(JsTTDMoveMode::JsTTDMoveKthEvent | ((int64) startEventCount) << 32);

Looks current JsTTDMoveMode needs 64 bits.
@mrkmarron

@jianchun good catch.  I found the line you quoted above https://github.com/Microsoft/ChakraCore/blob/master/bin/ch/ch.cpp#L275.  I did a little investigating and found that JsTTDMoveMode was hijacked to also pass in an event count.  You can see it being used at https://github.com/Microsoft/ChakraCore/blob/master/lib/Jsrt/Jsrt.cpp#L3781.  It appears that JsTTDGetSnapTimeTopLevelEventMove() is the only place JsTTDMoveMode is used in this way and https://github.com/Microsoft/ChakraCore/blob/master/bin/ch/ch.cpp#L275 is the only place it's ever called like this.  In fact, this is the only place JsTTDGetSnapTimeTopLevelEventMove() is called.
Abusing JsTTDMoveMode like this seems like a bad idea since it is very likely to cause confusion.  It's not at all obvious from looking at the definition of JsTTDMoveMode that it is going to be used in this way.  The easiest solution is to add a kthEvent parameter to JsTTDGetSnapTimeTopLevelEventMove().  I've implemented this change.

Hi @jcoffland, you are 100% correct that I was doing some parameter hijacking. I agree with you that this is suboptimal and converting the API to use a second parameter instead is more appropriate (thanks for your help cleaning this up).
One comment I have is that kthEvent is an optional parameter so we should probably make that clear in the SAL annotation/name.
@jianchun we will need to update the use of this API in the node/xplat branch when we merge later as well.

@mrkmarron right.  I've now changed the parameter annotation from _In_ to _In_opt.

Is it possible to add a sort of test suite which exercises the use of our API when embedding ChakraCore in C99 and C++98 projects?
On that note, @liminzhu, I remember someone mentioning that one or more samples had broken when we updated the API. Should we think about including a set of tests that include the samples?
Should this fix be back-ported to release/1.4, release/1.3, and/or release/1.2 because we still support 1.2 and 1.3, and we had intended this to work?

On another note:
Should this fix be back-ported to release/1.4, release/1.3, and/or release/1.2 because we still support 1.2 and 1.3, and we had intended this to work?

It wasn't an issue in 1.3 when I tried it.

@jcoffland Thanks for confirming that. In any case, I think this PR should probably be re-targeted to 1.4 (because I'm pretty sure this issue would affect that release). But, hold off on that until we confirm. Worst-case we can just merge to master and then back-port the change to 1.4.

I remember someone mentioning that one or more samples had broken when we updated the API. Should we think about including a set of tests that include the samples to ensure we don't unexpectedly break anything?

@dilijev We already test that for some time. See https://github.com/Microsoft/ChakraCore/tree/master/test/native-tests

@obastemur Do those tests check that the samples in https://github.com/Microsoft/Chakra-Samples work?

@jcoffland AFAIK we do check a bit more here. However, we should add more tests there indeed.

It would make sense to trigger CI builds on https://github.com/Microsoft/Chakra-Samples to make sure they build.  It would be best to have CI build badges on that project's README.md.  The reason being that the samples are one of the first places new users start with ChakraCore.  If it doesn't work they are likely to go elsewhere.

It would make sense to trigger CI builds on https://github.com/Microsoft/Chakra-Samples to make sure they build.

I am not sure if I was being clear. We already build and test the Jsrt usage (with the same xpalt compile scripts and same code) on ChakraCore CI for each PR.
If CI here is not failing, it is unlikely that we break Chakra-Samples. That is what https://github.com/Microsoft/ChakraCore/tree/master/test/native-tests for.

@obastemur What keeps native-tests in sync with Chakra-Samples?  My experience when I tried building Chakra-Samples was that they would not compile against ChakraCore master.  This makes for a disheartening introduction.
Anyway, I've gone off topic.  Can this PR be merged now?

What keeps native-tests in sync with Chakra-Samples?

We do. Anything is failing on native tests, should alert devs to update Chakra Samples. IMHO it is similar to updating documentation.

Can this PR be merged now?

Sorry for keeping this PR open for longer than it should. @dilijev any update?
In the mean time #2136 to make sure we don't break it again.

should alert devs to update Chakra Samples

@obastemur while this seems true in principle, I think it is easy to miss, and since, unlike documentation, Samples are something which we can actually break, maybe we should seriously think about adding some automated test mechanism to ensure they all continue to work.

Assigning myself. I'll test and merge.

It should be noted that there are additional compile problems using the Jsrt API with the MSVC compiler in C mode that are not addressed by this PR.
Thanks for the merge!

@jcoffland Thanks for the heads-up. Could you open an issue so we can track that?


'Expression did not evaluate to a constant' error when compiling latest tvm 0.9 on Windows 10

Hello,
I'm trying to compile latest tvm on Windows 10 / Visual Studio Community 2019 with USE_CUDA, USE_LLVM "llvm-config --link-static", USE MKL, USE_MKLDNN, USE_CUDNN, USE_DNNL_CODEGEN, USE_TENSORRT_CODEGEN, USE_TENSORRT_RUNTIME options (cmake generate solution files successfully without any error) but it failed with following error when compiling in VS2019:-
2>D:\code\tvm\src\runtime\contrib\dnnl\dnnl_json_runtime.cc(609,18): error C2131: expression did not evaluate to a constant
2>D:\code\tvm\src\runtime\contrib\dnnl\dnnl_json_runtime.cc(609,18): message : failure was caused by a read of a variable outside its lifetime
2>D:\code\tvm\src\runtime\contrib\dnnl\dnnl_json_runtime.cc(609,18): message : see usage of 'OC'
Then later a lot of link errors.... i.e.
4>LINK : fatal error LNK1181: cannot open input file 'D:\code\tvm\build\tvm_runtime_objs.dir\Release\src\runtime\vm\vm.cc.obj'
8>LINK : warning LNK4044: unrecognized option '/Wl,--exclude-libs,ALL'; ignored
7>LINK : warning LNK4044: unrecognized option '/Wl,--exclude-libs,ALL'; ignored
8>LINK : fatal error LNK1181: cannot open input file 'D:\code\tvm\build\tvm_objs.dir\Release\src\relay\backend\contrib\tensorrt\codegen.cc.obj'
7>LINK : fatal error LNK1181: cannot open input file 'D:\code\tvm\build\tvm_objs.dir\Release\src\relay\backend\contrib\tensorrt\codegen.cc.obj'
I don't know much of C++ but apparently error 'expression did not evaluate to a constant' in \src\runtime\contrib\dnnl*dnnl_json_runtime.cc line 609* is justified as on same line  float bias[OC] = {0}; OC is not a constant which compiler expects at compile time.
If respective developer of the dnnl codegen module can look into it?
Thanks for all the great work.

That warning is a legit one, https://github.com/apache/tvm/blob/45f3d4a521ec476cd9960e3d2de4f66bde61bf23/src/runtime/contrib/dnnl/dnnl_json_runtime.cc#L609-L610
OC is indeed not a constant.
Instead it should be
      std::vector<float> bias(OC, 0);
      write_to_dnnl_memory(bias.data(), bias_memory, OC * sizeof(float));

Can you try this on windows, and send a PR if it solves the issue?

Thanks dear,
it worked like a charm...no compilation error now. However, i guess there is another compiler / linking error (seems small one)
Error	D8021	invalid numeric argument '/Wno-deprecated-declarations'	tvm_runtime_objs	\tvm\build\cl
I guess it is just extra commandline option (which is not recognized by VS2019 cl.exe) being passed to compiler however, i don't seem to find out the way how to disable / remove it. Tried looking in solution & project properties\commandline options but to no avail.
Can you please look into as well...
Thanks

Are you enabling TensorRT with TVM? It looks like no-deprecated-declarations is only used by TensorRT backend. Probably our TensorRT support doesn't work on Windows.

That's correct. I'm trying to compile it with TensorRT. By looking at tvm_runtime_objs.vcxproj in VSCode, i found following lines:-

%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations

%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations

%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations
%(AdditionalOptions) -Wno-deprecated-declarations

As i understand, VS cl.exe doesn't understand these flags so i simply removed these. So far compilation is going smooth after that. I will update once it completes.
Thanks

I can confirm that compilation went smooth with std::vector suggestion by @masahi . For -Wno-deprecated-declaration related errors, i modified tvm\cmake\modules\contrib\TensorRT.cmake file and commented out lines similar to
set_source_files_properties(${COMPILER_TENSORRT_SRCS} PROPERTIES COMPILE_FLAGS "-Wno-deprecated-declarations")
Compilation went OK, able to install tvm python package and tvmc seems to work. Now trying to setup python unittests to verify if everything is OK. You may close this issue now.
Thanks


opencv3.0 deepflow

I have compared the code of deepflow given in current opencv and the code provided by the author, I found that current code in opencv does not include the deepmatching component, when will it be finished?
Thanks!

Deepflow is part of the opencv_contrib repository and thus this issue does not belong here. That being said it has already been reported there https://github.com/Itseez/opencv_contrib/issues/319 and we already answered why it is not in yet and why it will not ever be in...


[downgrade] stage-level environment variables do not convert correctly

Here is an example v1 yaml
stages:
- name: test
  spec:
    envs:
      STAGE_VARIABLE: value
    platform:
      arch: amd64
      os: linux
    runtime:
      spec: {}
      type: cloud
    steps:
    - name: hello
      spec:
        image: busybox
        run: |
          echo hello
        shell: bash
      type: script
  type: ci
version: 1

which converts to this v0 yaml
pipeline:
  identifier: default
  name: default
  orgIdentifier: default
  projectIdentifier: default
  properties:
    ci:
      codebase:
        build: <+input>
  stages:
  - stage:
      identifier: test
      name: test
      spec:
        cloneCodebase: true
        execution:
          steps:
          - step:
              envVariables:
                STAGE_VARIABLE: value
              identifier: hello
              name: hello
              spec:
                command: |
                  echo hello
                image: busybox
                shell: Bash
              timeout: ""
              type: Run
        platform:
          arch: Amd64
          os: Linux
        runtime:
          spec: {}
          type: Cloud
      type: CI

envVariables is not valid at this level, STAGE_VARIABLE is not available to the hello step shell commands.
It should convert to this v0 yaml
pipeline:
  identifier: default
  name: default
  orgIdentifier: default
  projectIdentifier: default
  properties:
    ci:
      codebase:
        build: <+input>
  stages:
  - stage:
      identifier: test
      name: test
      spec:
        cloneCodebase: true
        execution:
          steps:
          - step:
              identifier: hello
              name: hello
              spec:
                command: |
                  echo hello
                image: busybox
                shell: Bash
              timeout: ""
              type: Run
        platform:
          arch: Amd64
          os: Linux
        runtime:
          spec: {}
          type: Cloud
      type: CI
      variables:
      - name: STAGE_VARIABLE
        type: String
        description: ""
        required: false
        value: value

Valid values for type are String, Secret and Number, I think we only need to worry about String and Number. We might be able to get away with treating everything as String with double quotes and not worry about Number.

Fix in #140


Blueprint compilation support

Supports compilation of exported blueprints, and nesting blueprints inside of behaviors.  Behaviors can also be nested inside of blueprints.
Please see the tests/blueprint_with_behavior.ts file for a showcase of the supported features.

@ribrdb I went ahead and merged this. If you did have any further feedback or run into issues feel free to let me know and I'll look into it.


A less dom and more template approach

Currently the way the rsx! macro parses and outputs something akin to a psuedo-dom style set of element and attributes is similar to what React does. But the current use case of this crate is server side rendering (unless you have plans for this with wasm), a dom like structure is an implementation detail. With this in mind, it would be more performant to pass thru most of the template as static bytes to write calls, and splice in dynamic content where we find it.
As an example, for this component:
#[component]
fn Container<'kind, Children: Render>(
    kind: &'kind str,
    children: Children,
) {
    rsx! {

          {children}

    }
}

Old output:
let result = {
    let Container { kind, children } = self;
    {
        ::render::SimpleElement {
            tag_name: "div",
            attributes: {
                let mut hm = std::collections::HashMap::<&str, &str>::new();
                hm.insert("class", kind);
                Some(hm)
            },
            contents: Some(children),
        }
    }
};
::render::Render::render_into(result, w)

New output:
let Container { kind, children } = self;
write!(w, b"
")?;
::render::Render::render_into(children, w)?;
write!(w, b"
")?;
Ok(())

I think there are other things to consider with this change, like automatic quoting of attribute, punning (which I independently am unsure of) etc. I'll figure that out as I make progress. I'm going to experiment with this idea and see how far I can take it without running into unforeseen issues.
Thoughts?

it would be more performant to pass thru most of the template as static bytes to write calls

That's probably true but I wonder if that can be optimized by the compiler? I'm not sure how strong it can get 😃 because SimpleElement output should be kinda similar to the "new output". I made the SimpleElement struct because it was easier for me, but if we can see a real performance gain we can maybe just write the bytes directly!
Regarding DOM-specific things, I agree that it might be weird to have this ideas baked in, but do you agree that it would be mostly used as an HTML template generator having this special case is better?

If you want to have a "custom component" that renders whatever you want, you can use a struct/"function component". I imagine it like React Native, where you don't use lower case components at all. Everything is a "custom component", whether provided by the user or by the framework itself
You can always return a string because it is Render
You can always just write! if you want to (like we do internally 😄)
As you mentioned the quoting, we also escape the strings, which I'm not sure we should always do, but probably need to do that automatically for HTML elements' attributes and (string) children so XSS won't be an issue if you use Render with a web framework

Also, lately I found maud which returns an owned string. Owned strings should be less efficient, memory wise, than what we do. But they say that practically it is faster. I don't think we should micro-optimize things that will hurt expressiveness and ease of use. What do you think about that?
(It's sometimes a problem to express feeling in writing, especially when having a language-barrier. Just to be on the safe side, These are genuine questions, would be happy to get your feedback 😃)

That's probably true but I wonder if that can be optimized by the compiler? I'm not sure how strong it can get smiley because SimpleElement output should be kinda similar to the "new output". I made the SimpleElement struct because it was easier for me, but if we can see a real performance gain we can maybe just write the bytes directly!

I'm pretty sure the compiler can't optimize away the use of the HashMap / HashSet -- but I'm no expert, will need to investigate.

Regarding DOM-specific things, I agree that it might be weird to have this ideas baked in, but do you agree that it would be mostly used as an HTML template generator having this special case is better?

To be clear, I'm not suggesting supporting non HTML things -- I'm just talking about the internal implementation to prefer passing bytes thru rather than parsing individual tags into nodes etc.

If you want to have a "custom component" that renders whatever you want, you can use a struct/"function component". I imagine it like React Native, where you don't use lower case components at all. Everything is a "custom component", whether provided by the user or by the framework itself

With what I'm suggesting, the way to write components will not change. In fact, I'm pretty sure most current code will just keep working (only punning is what I'm unsure of).

Also, lately I found maud which returns an owned string. Owned strings should be less efficient, memory wise, than what we do. But they say that practically it is faster.

This is interesting. I'm not sure what they tested exactly, but would be interesting to find some benchmarks and get clarity on the testing.

I don't think we should micro-optimize things that will hurt expressiveness and ease of use. What do you think about that?

Again I want to be clear that the ergonomics of using the library will not be affected by what I'm proposing!

(It's sometimes a problem to express feeling in writing, especially when having a language-barrier. Just to be on the safe side, These are genuine questions, would be happy to get your feedback smiley)

Absolutely! It's good to discuss these things and thanks for letting me know English isn't your primary language :) Feel free to be frank and direct!
I'm going to experiment with it, because I'm still convinced it will be easier/faster/better in various ways. I'll also write some benchmarks so we can get some real numbers to compare as well. If it doesn't work out that way, I'll be fine to discard the work :)

To be clear, I'm not suggesting supporting non HTML things -- I'm just talking about the internal implementation to prefer passing bytes thru rather than parsing individual tags into nodes etc.

Ah, that sums it up 😄 basically inlining SimpleElement, no? I think it can be good 😄
But if that's too difficult and harder to test, maybe we can drop the HashMap/HashSet usages in favor of arrays of tuples (like I thought in #6)? Not sure if this can be optimized away either but it feels lighter 😄
I'll be happy to see the results 👏

Ah, that sums it up smile basically inlining SimpleElement, no? I think it can be good :smile:

Yes, basically!

But if that's too difficult and harder to test, maybe we can drop the HashMap/HashSet usages in favor of arrays of tuples (like I thought in #6)? Not sure if this can be optimized away either but it feels lighter :smile:

That might actually be something the compiler can optimize away!


[id] add sudo to Red Hat-based distributions

The commands needs to be run using sudo.
Debian-based distributions already include sudo in the documentation, but Red Hat-Based distributions, so I added it.

We can close the PR if it's not no upstream English change. Or I would
suggest changing in English first with the proposed changes if required,
then we can get back to this. Closing for now.
/close
On Thu, 28 Oct 2021 at 03.13 Tim Bannister @.***> wrote:

@.**** requested changes on this pull request.
This change is not based on an upstream English change. We should not
merge it.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/website/pull/30254#pullrequestreview-791111042,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYLMG43C2ZF5OYUFBBJII3UJBTP5ANCNFSM5GYVWMEA
.

@girikuncoro Couldn't you approve the EN one (https://github.com/kubernetes/website/pull/30247) instead of closing this one?
@sftim  is not responding a couple a days. @girikuncoro  @phanama  Do you know anyone else who can approve the EN one?

@girikuncoro  the en PR (https://github.com/kubernetes/website/pull/30247) is approved and merged. Would you proceed this?

thanks a lot for this @aysabzevar
/approve
/lgtm

thanks a lot for this @aysabzevar
/approve
/lgtm

/remove-lifecycle stale


Add backwards compatibility to maya.ls

This should take care of listing assets loaded with an older version of the pipeline.

I'm having trouble with this.
The problem is the containers in the scene has changed their contents.
From..
{
  "id": "",
  "name": "",
  "author": "",
  "loader": "",
  "families": "",
  "time": "",
  "asset": "",
  "subset": "",
  "representation": "",
  "version": 0,
  "silo": "",
  "path": "",
  "source": ""
}

To..
{
  "id": "",
  "name": "",
  "namespace": "",
  "loader": "",
  "project": "",
  "asset": "",
  "subset": "",
  "version": 0,
  "representation": ObjectId()
}

The critical differences is that representation is the ID to the object in the database, it's used to find relevant versions to the original.
This is one of those cases where implementing backwards compatibility is possible, but introduces a lot of code..

Actually, with "representation" being the ObjectId wouldn't asset and subset actually be irrelevant information since they are based on the representation.

Actually, with "representation" being the ObjectId wouldn't asset and subset actually be irrelevant information since they are based on the representation.

They might. The original reason for adding so many members, was to not have to query the database so much. An optimisation.
I might still opt for this route, as currently the one thing that requires a unique database query is checking versions, and that could be checked more explicitly, e.g. a global button "check for updates" or an individual right-click menu item.
It'd enable us to make a more performant application, and database, in one swoop.
For example, and we should talk more about this once things settle, the new loader is 64x slower than the original (0.05s versus 3.5s on a complex, real world production assets) due to the excessive queries.

For example, and we should talk more about this once things settle, the new loader is 64x slower than the original (0.05s versus 3.5s on a complex, real world production assets) due to the excessive queries.

Interesting. Note that the queries it does now are upon refresh on all the subsets at once, instead of the a single "subset", then a single "representation", etc. Plus to also know whether it is the latest version we'd need to know about the other versions. Did you test both now, or are you comparing to older times you got from your loader? (If so, that one will now likely be slower because of some backwards compatible code recently introduced)
(3.5 seconds sounds really slow by the way! With the new loader which is close to our loader used without database even in scenes with over 10.000 objects (many similar ones though) I think it was faster than that, which was using cquery.)
But definitely, we should be looking forward and get good speeds.

Ok, here's how I'm thinking we tackle this.
Containers in Maya have a fixed set of data, very similar to any other document in the database. For documents in the database, each is imbued with a schema value specifying what format the data is in.
In the case of containers in Maya, this doesn't exist. Which means we can't implement functionality to detect the format before handling it.
So considering we'll likely keep changing this, and should be able to change it, I'll consider this and the past versions as 1.0 and 2.0 respectively, and embed a schema value into the containers created with this latest update. Since no artist is yet using this in production, it is safe to make this change.
Then, when there is a schema attribute on new containers, and no schema attribute on the previous version, we have an explicit method of detecting the schema.
For following versions, we can increment the version of the schema, and thereby enable change of containers indefinitely, including scenes with containers at mixed schema versions.
The solution is complex, but so is the problem I think. Any ideas, let me know.

Then the question is whether we should transform the data coming from Maya before making use of it e.g. the Manager GUI, or whether we let the Manager be the one to handle multiple versions of schemas.
For example. If a scene contains two loaded assets of one version each. The resulting dictionaries fed into the Manager could be either the data as-is, or pre-transformed into a common format such that the GUI would never need to change.
This pre-transformation would then need an additional schema - an "operator" schema, if you will - , separate from the "data" schemas.
I would say it's best to let GUIs handle this themselves. Missing or unsupported functionality could then be properly visualised in the GUI. They could potentially provide their own "migrate" functionality where possible.

To give you a picture of what the above was about.

 Missing or unsupported functionality could then be properly visualised in the GUI. They could potentially provide their own "migrate" functionality where possible.

Having this visually there might also introduce some more complexity. I'm mostly afraid that even though the manager might know how to handle it the API itself doesn't.
The important bit I think is that the API returns as much consistent data as we can, as such the API should already return it... right?

The important bit I think is that the API returns as much consistent data as we can, as such the API should already return it... right?

It would, in that data is handed to you in dictionary form, but their internal layout would depend on which version of the schema they adhere to.
This is unavoidable, and one of the things we can hold against an SQL approach where migration would be difficult or impossible.
Consider what would happen if we didn't allow for containers with data of two different schemas.
container1
{
  "name": "Joe"
}

container2
{
  "nickname": "Joe"
}

If we made this change, we would need to update our tools, like Manager, to account for it. But, we would have no control over what the data would look like in containers produced before this change.
The only option is to migrate this data. But we can't, because the data is embedded into the workfiles of artists. Migrating it would involve opening each scene and remove/add attributes from the containers themselves. Only at that point would our tools, like Manager, be able to work with them.
With this document-database oriented approach with embedded schemas, we can now facilitate data in different schemas, without requiring migration.
Neither approach is not without complexity. But it'll have to be one or the other, because change is inevitable.

Still. I would say api.ls() would convert the data to a given format.
As such api.ls() would return it both as name if that's the newer format. But I'm not sure whether that would break other things. But otherwise it'll be hard to ever write tools in the future, because you'll need to know about the older data structures, otherwise the api would manage that?

Looking at this I can see that it might keep things "closer" to the end functionality for backwards compatibility, but this would introduce the same backward compatibility functionality needs to be implemented per tool. (And I can see that number grow rapidly as time goes on?)

because you'll need to know about the older data structures

Yes, we will. And we do, this is what the schemas are for. They explicitly define what is available in the data we make use of.

but this would introduce the same backward compatibility functionality needs to be implemented per tool. (And I can see that number grow rapidly as time goes on?)

This is what I meant that either approach will involve complexity.
Let's consider the alternative, rather than have each tool support multiple schemas, we pre-process all data to conform to a single schema. That is, Option 2 above.
Let's use the layout we use currently in the new Manager, which looks like this.
{
  "representation": "5fj29ytbjdej3hj"
}

Where the representation is a MongoDB ID that we use from within the Manager to figure out (1) asset, (2) subset and (3) version.
1 month later
Let's consider a hypothetical example, where we discovered that going this path resulted in a performance penalty for shots with many containers and would like to improve performance by embedding (1) asset, (2) subset and (3) into the container itself.
{
  "asset": "Bruce",
  "subset": "modelDefault",
  "version": 5,
  "representation": "ma"

Now there is no need to interact with the database as all of the information would reside in the local Maya scene. Great.
However, because maya.ls() has been designed to always return the same results, regardless of the data it is given, it would now have to convert these attributes into a MongoDB ObjectId before passing it onto the Manager, who will be expecting it.
converted = {
  "representation": io.locate(api.session["project"], asset, subset, version, representation)
}

Now the Manager can continue working without modification, but look. Now we've made things even slower by introducing an additional query to the database for the transformation of the data meant to speed things up.
6 months later
We discover that MongoDB doesn't scale and decide to go for PostgreSQL. Suddently the ObjectId we use for the representation in the data returned by maya.ls() isn't relevant anymore so we must change it.
But look, we can't, because the Manager has been designed to use this ID to send queries to MongoDB. What we'll have to do, is refactor the results returned by maya.ls() so that it can work with the new database. And in doing so, remove support for projects using MongoDB.
The end result is that we can't transition between the two databases, but must perform a "clean switch", such as from one project to another. In reality this won't happen and would likely become a problem.

This is consuming more energy than it's worth.
Let's stick with this breaking change, and have artists at Mindbender run an optional migration script if needed. From now on I expect every change to the pipeline to be fully backwards compatible. This means building things in such a way that multiple versions of each schema can be maintained.
For example.
# Don't
for container in maya.ls():
  print(container["some_member"]

# Do
for container in maya.ls():
  # "schema" is a required member for all container versions
  if container["schema"] == "avalon-core:container-1.0":
    handle_1_0(container)
  elif container["schema"] == "avalon-core:container-1.0":
    handle_2_0(container)
  else:
    raise UnsupportedSchema("%s is not supported" % container["schema"])

As discussed above, there's complexity involved either way, so let's give this route a go and see how things turn out. It'll be a lot easier to revert a system with backwards compatibility to one without, than the other way around.