Text Record Presentation Header Includes Space For Row Number Column

System information:

DBeaver version <IP_ADDRESS>205011839

Describe the problem you're observing:
"Record" view text presentation header incorrectly includes space for row number.
Steps to reproduce, if exist:
Window >> Preferences >> Editors >> Data Editor >> Appearance >> Plain text >> Check the "Show line numbers" box.
Run this script and switch to Text mode (CTRL+`):
select * from (values (1,'one','I'),(2,'two','II')) t("number",written,roman);

Results:
| # | number | written | roman |
+---+--------+---------+-------+
| 1 |      1 | one     | I     |
| 2 |      2 | two     | II    |

Now switch to "Record" mode (Tab) and see that the header/data delineation line still includes space for the row number (the leading +---):
| Name    | Value |
+---+---------+-------+
| number  | 1     |
| written | one   |
| roman   | I     |

@jetzerb, Hi. Is this still reproducible in the recent version of DBeaver?

@Destrolaric : No, it's no longer an issue.  Not sure when it was fixed, but I'm currently running 22.2.5 and the text displays correctly.


Fixed typo

BTW Dylan I have just reset my fork repo and I saw that you were watching so you need to click "watch" again.

Thanks!


S3 trigger event does not work in the mock mode

Before opening, please confirm:

[X] I have installed the latest version of the Amplify CLI (see above), and confirmed that the issue still persists.
[X] I have searched for duplicate or closed issues.
[X] I have read the guide for submitting bug reports.
[X] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

How did you install the Amplify CLI?
npm
If applicable, what version of Node.js are you using?
v14.18.1
Amplify CLI Version
7.5.3
What operating system are you using?
Mac
Amplify Categories
storage
Amplify Commands
Not applicable
Describe the bug
When I uploaded a photo using Storage.put({identityId: 'xxxxx', ...}) in amplify mock mode, the amplify mock command crashed with the following error.
2021-11-28T13:14:48.871Z - error: uncaughtException: The "path" argument must be of type string. Received undefined
:
stack=[TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined,
at new NodeError (internal/errors.js:322:7),
at validateString (internal/validators.js:124:11),
at Object.join (path.js:1148:7),
at loadLambdaConfig (/Users/hoge/.nvm/versions/node/v14.18.1/lib/node_modules/@aws-amplify/cli/node_modules/amplify-util-mock/src/utils/lambda/load-lambda-config.ts:27:29),
at runMicrotasks (<anonymous>),
at processTicksAndRejections (internal/process/task_queues.js:95:5),
at StorageServer.<anonymous> (/Users/hoge/.nvm/versions/node/v14.18.1/lib/node_modules/@aws-amplify/cli/node_modules/amplify-util-mock/src/storage/storage.ts:133:22)]

This error occurred after migrating to amplify-cli ver7. (https://docs.amplify.aws/cli/migration/override/)
In the previous version, S3 trigger event worked properly.
My cloudformation template file(amplify/backend/storage/hoge/s3-cloudformation-template.json) was as follows.
amplify/backend/storage/hoge/s3-cloudformation-template.json
:
                "NotificationConfiguration": {
                    "LambdaConfigurations": [
                        {
                            "Event": "s3:ObjectCreated:*",
                            "Function": {
                                "Ref": "functionS3Trigger51e7dea0Arn"
                            }
                        }
                    ]
                },
:

The value of triggerName variable was assigned here in this template file.
--
In the latest version(v7.5.3), my cloudformation template file(amplify/backend/storage/hoge/build/cloudformation-template.json) was as follows.
amplify/backend/storage/hoge/build/cloudformation-template.json
"NotificationConfiguration": {
          "LambdaConfigurations": [
            {
              "Event": "s3:ObjectCreated:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "protected/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            },
            {
              "Event": "s3:ObjectCreated:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "private/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            },
            {
              "Event": "s3:ObjectCreated:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "public/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            },
            {
              "Event": "s3:ObjectRemoved:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "protected/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            },
            {
              "Event": "s3:ObjectRemoved:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "private/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            },
            {
              "Event": "s3:ObjectRemoved:*",
              "Filter": {
                "S3Key": {
                  "Rules": [
                    {
                      "Name": "prefix",
                      "Value": {
                        "Fn::Join": [
                          "",
                          [
                            "public/",
                            {
                              "Ref": "AWS::Region"
                            }
                          ]
                        ]
                      }
                    }
                  ]
                }
              },
              "Function": {
                "Ref": "functionS3Trigger51e7dea0Arn"
              }
            }
          ]
        }

As far as I investigated, the value of node variable was assigned here.
But keyName did not include the region.
So this line was never executed and triggerName was never assigned.
As a result, the error occurred.
In my local environment, I made the following modifications, and the error was resolved.
Index: packages/amplify-util-mock/src/storage/storage.ts
<+>UTF-8
===================================================================
diff --git a/packages/amplify-util-mock/src/storage/storage.ts b/packages/amplify-util-mock/src/storage/storage.ts
--- a/packages/amplify-util-mock/src/storage/storage.ts	(revision b6504ca858be264fbb19b377c9a38343cbb7c313)
+++ b/packages/amplify-util-mock/src/storage/storage.ts	(date<PHONE_NUMBER>919)
@@ -112,7 +112,7 @@
           for (let rules of prefix_arr) {
             let node;
             if (typeof rules.Value === 'object') {
-              node = String(Object.values(rules.Value)[0][1][0] + String(region) + ':');
+              node = String(Object.values(rules.Value)[0][1][0]);
             }

             if (typeof rules.Value === 'string') {

Expected behavior
Lambda function is executed by S3 trigger event.
Reproduction steps
Please see Describe the bug section.
GraphQL schema(s)
No response
Log output
No response
Additional information
No response

Hi @laboone, thank you for raising this issue. I marked it as a bug for the team to pick up.

@laboone
The following PR is being raised to fix the change in the notificationTrigger template .
https://github.com/aws-amplify/amplify-cli/pull/9235
an example of the policy after the fix is:
"NotificationConfiguration": {
"LambdaConfigurations": [
{
"Event": "s3:ObjectCreated:",
"Function": {
"Ref": "functionnewtrigger87c981caArn"
}
},
{
"Event": "s3:ObjectRemoved:",
"Function": {
"Ref": "functionnewtrigger87c981caArn"
}
}
]
}
Hence the following will continue to work with the next release:
String(obj.Function.Ref).split('function')[1].split('Arn')[0];

Closing this, since fix has been merged.


Fixes last isues before publishing

Closes #26 #46 #47 #30 #43 #44
📑 Description

✅ Checks

[x] My pull request adheres to the code style of this project
[x] My code requires changes to the documentation
[x] I have updated the documentation as required
[x] All the tests have passed
[x] I have selected a committee co-chair to review the PR

ℹ Additional Information

@vassilpeytchev please close any issues that this PR has addressed.

I will merge this and review in master. @MaryLJ will pick up with that on tuesday. Hope to get this into next week Thursday release.


All genes page

Hi,
The all_genes pages seems to load but doesn't show all of the genes.
Thank you,
Lynn

The length of all_genes is less than the length of all_ids because there are some gene names which are present in both TCGA Tier 3 mRNAs and MiTranscriptome beta lncrnas, whereas every id is unique.


Move @types packages to dependencies from devDependencies

Bug report

[x] I have checked other issues to make sure this is not a duplicate.

Describe the bug
I have a following error after running tsc --noEmit:
error TS7016: Could not find a declaration file for module 'yup'

This should be fixed by moving all @types modules to dependencies, not devDependencies (I should not install type definitions for package that is not my dependency.

I believe there's an underlying issue to your problem. From my experience, TS libraries pack type definitions in development dependencies because they are part of the development environment. Would you mind composing a short reproduction repository?

You are right that they are part of development experience - that's why they should be in production dependencies, as devDependencies are not installed. apollo-client, for example ships third party types definitions in dependencies here.
@types/module-x packages are in devDependencies when module-x dependency is declared in peerDependencies. In that case, I, as a consumer, am responsible for installing said package (plus @types package). In this case, I haven't installed yup, therefore I should not need to install @types/yup (so my typecheck script won't fail).
Hope this helps, great job with this module btw, loving it :)

Thanks a lot! The reason I am so reluctant with the change is that I have never experienced the bug myself.
If I understand the issue correctly, we should only move @types/yup to dependencies because we use yup during development.
Do you by chance have any other reference to a similar issue?

Short answer is yes.
Files src/constructors.ts and src/rules.ts export stuff that uses yup types, therefore they (@types/yup) should be available to the consumer (installing them as production dependencies). Example:
export const inputRule = <T>(
  name: string | ((yup: typeof Yup) => Yup.Schema<T>),
  schema?: (yup: typeof Yup) => Yup.Schema<T>,
) => {
  if (typeof name === 'string') {
    return new InputRule(name, schema(Yup))
  } else {
    return new InputRule(Math.random().toString(), name(Yup))
  }
}

Now, let's say I use inputRule inside my code. My typescript doesn't know what Yup.schema<T> or typeof Yup means, because I don't have @types/yup package installed (as I don't directly use yup package). Hope this make sense. This is a longer answer, because you don't use yup only during development, but also in the distributed package.

Makes sense now. I will implement the fix by the end of the week.
If you have some spare time, I could merge a PR as soon as it lands.

:tada: This issue has been resolved in version 5.3.2 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Working example for v2.4.1

I have an existing app that I am trying to get working with your plugin for flutter_map. I can only use version 2.4.1 of your plugin & v2.2.0 of flutter_map (without having to significantly rework the app) and wondered if you had a working example of how I can show mapbox vector tiles and a rain layer.
The style/theme link I am using is:
https://api.mapbox.com/styles/v1/bom-dc-prod/cl82p806e000b15q6o92eppcb?access_token=ACCESS_TOKEN
And the url for the rain radar layers is:
https://api.mapbox.com/v4/bom-dc-prod.rain-prod-LPR-{time}/{z}/{x}/{y}.vector.pbf?access_token=ACCESS_TOKEN
I can get the map showing using the latest version of your plugin (but not sure how to also add the rain radar layer), but if I use 2.4.1 there is no Style class so I’m struggling to get the theme. I’ve been trying to get this working for some time, so would really appreciate your help.
Thankyou!

Your best bet may be to reconsider upgrading flutter_map and this plugin to the latest version. Though it will involve rework in your app, it may be less effort and more future-proof than trying to get older versions to work.
If you want to get a Mapbox style to work with 2.4.1, you could back-port StyleReader
To add the rain layer, you could add another layer as a child of FlutterMap, e.g. here https://github.com/greensopinion/flutter-vector-map-tiles/blob/main/example/lib/main.dart#L107
You'd need to figure out how to make the additional layer transparent so that the map layer underneath shows through. If you use a vector tile layer from this plugin, you'd want to have a map style that has no background layer.
A good place to look for examples is the flutter-vector-map-tiles-examples project - thought they are specific to the latest version of this plugin.

What did you end up doing?
On Fri, Apr 21, 2023 at 5:52 AM roly151 @.***> wrote:

Closed #138
https://github.com/greensopinion/flutter-vector-map-tiles/issues/138 as
completed.
—
Reply to this email directly, view it on GitHub
https://github.com/greensopinion/flutter-vector-map-tiles/issues/138#event-9067432208,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEKDQ7VBYGVAS7HNZD6HVSDXCJ7KLANCNFSM6AAAAAAWXC2XXA
.
You are receiving this because you commented.Message ID:
<greensopinion/flutter-vector-map-tiles/issue/138/issue_event/9067432208@
github.com>

I tried to implement as per your advice, but in the end changed to a dedicated mapbox package. I will look to rework the app, but I needed to get a fix out asap. Thankyou very much for the advice though.

Sounds good! Thanks for letting me know.
On Sun, Apr 23, 2023 at 7:19 PM roly151 @.***> wrote:

I tried to implement as per your advice, but in the end changed to a
dedicated mapbox package. I will look to rework the app, but I needed to
get a fix out asap. Thankyou very much for the advice though.
—
Reply to this email directly, view it on GitHub
https://github.com/greensopinion/flutter-vector-map-tiles/issues/138#issuecomment-1519289578,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEKDQ7UXKAMJADZJYIEPEYLXCXPJVANCNFSM6AAAAAAWXC2XXA
.
You are receiving this because you commented.Message ID:
@.***>

Hi Again, I finally found time to rework my app and I am now using the latest version of flutter_map and your plugin. I am hoping you can help me understand how to add the rain radar layer. The map loads with the first VectorTileLayer, but the second VectorTileLayer which is loading the rain radar data seems to overwrite the map. You mentioned previously about using a style with a transparent background. I would assume that the style I am using would have this. But I am not experienced enough to know how to find out. I am happy to share my style token privately if you are able to help?
The style/theme link I am using is:
https://api.mapbox.com/styles/v1/bom-dc-prod/cl82p806e000b15q6o92eppcb?access_token=ACCESS_TOKEN
And the url for the rain radar layers is:
https://api.mapbox.com/v4/bom-dc-prod.rain-prod-LPR-{time}/{z}/{x}/{y}.vector.pbf?access_token=ACCESS_TOKEN

I'm sorry that I can't look into it right now. Feel free to dig into it yourself! It might help to look for a background layer in the style that you're using, or put a breakpoint here https://github.com/greensopinion/flutter-vector-map-tiles/blob/62b81acd8fe2cde32c436037f8161428aceafb09/lib/src/grid/tile/tile_painter.dart#L69

Thankyou for the tip, and appreciate your time. The style/theme url I am trying to use includes multiple layers. Is there a way to use a specific fill layer from my style? The layer I need to use is a fill-layer that has an id. Is it possible to reference the id to apply the fill properties provided by that layer to my VectorTileLayer? (Apologies if this is simple or I am not explaining it right. I am still trying to get my head around styles, themes, sources, layers etc).

I've provided a multi-layer example: https://github.com/greensopinion/flutter-vector-map-tiles-examples
See https://github.com/greensopinion/flutter-vector-map-tiles-examples/blob/main/lib/examples/multi_layer.dart for details.


Migrate cf-buttons to cfv4

Short description explaining the high-level reason for the pull request
Additions

cf-buttons styles moved temporarily to enhancements.

Changes

Updates outdated btn classes to a-btn.

Testing

./frontend.sh and site should look the same as prod.

Notes

Pagination disabled button background color is darker than the pagination background. This will have to be revisited when updating the pagination module.

Merging this as it's just a snapshot of a larger PR that'll come through cfv4_migration to master later on.

Merging this as it's just a snapshot of a larger PR that'll come through cfv4_migration to master later on.

I would avoid doing this. Reviewing that PR later is going to be a huge task as it is, it'll be easier to catch small things in these smaller PRs.

I would avoid doing this. Reviewing that PR later is going to be a huge task as it is, it'll be easier to catch small things in these smaller PRs.

The problem was I couldn't reliably continue work on the main branch for the migration while the PR was open, but feel free to comment on anything in the snapshot PRs. https://github.com/cfpb/cfgov-refresh/pull/3062 https://github.com/cfpb/cfgov-refresh/pull/3052 https://github.com/cfpb/cfgov-refresh/pull/3049


Updated to remove reporting error to Honeybadger if number of retries is...

... 0
https://thinkneardev.atlassian.net/browse/ES-1118

wait wait. I don't know about this code. :tada:

Hi @KamilMroczek

yooooo @yutamgit


admin panel must be protected by username and pwd

Not interesting for assigment. To do in a next release


USB joystick for Windows Subsystem for Linux?

I am using WSL to use ROS for a graduate course, and I need to access a joystick usb driver through WSL. I want to figure out how I can do that. When I plug in the joystick to my computer it doesn't show on /dev/. The joystick uses Windows' xusb22.sys driver, and it's showing that it's connected to Port_#0001.Hub_#0001. How can I access this joystick usb data through WSL?

How can I access this joystick usb data through WSL?

Unfortunately for your use-case, WSL doesn't have /dev/js? or block/character device support in general (with the exception of /dev/tttyS? and some special devices like /dev/null). We also don't have /dev/usb/* (libusb) support yet ref landing zone #412 and this User Voice. You could (in principle) open a new User Voice for /dev/js?, but if it were me I'd upvote libusb. Doing that won't get you joystick support before your course completes, but it does help express community interest in improved device support for WSL.


Create management command for posting SQS messages as issues to Github

Part of the work @willbarton and I are doing around alerts.  This is a very dumb first iteration of posting the SQS message body as an issue to Github, where the message body would be some type of Jenkins alert. This script will be called from Jenkins.
Adds two dependencies - boto3 for interfacing with the API for Amazon SQS, and github3 for interfacing with the Github API
Example usage
(note - the below will not work as is, you must replace the strings with the right credentials. If you'd like to test it I can provide the credentials):
python cfgov/manage.py post_sqs_messages_to_github --queue_url='https://sqs.queue-url' --aws_access_key_id='AWS access key' --aws_secret_access_key='AWS secret key' --github_token='Github Token' --github_url='Github Enterprise URL' --github_user='Username' --github_repo='Repo name'

If this is a management command instead of just a script, we're going to have to setup a virtualenv with all the cfgov-refresh requirements and everything in the Jenkins workspace that'll run the command.
We also don't need boto and github3.py installed on the production servers, just the Jenkins server, which is what'll happen also with those in the base requirements.
Can we make it just a script?

@willbarton Yes, I can make it into a script.  @chosak hmmmm, is v1 synonymous with wagtail? I'm happy to move it wherever, but maybe good to clarify directories.

@richaagarwal it's sort of grown into a catchall but I would love to put pure Python/Django things in core and have things more specific to Wagtail/cf.gov in v1.


BUG 1283: erlang_js uses non-thread-safe driver function

Using driver_output_term() in an async worker pool thread
is a big no-no.  A solution is to use driver_output_term()
in the ready_async() function, which executes in the proper
thread context.
Tested via:

Run with a debug SMP build of the Erlang VM.  There are no
beam/global.h:1345: Lock check assertion errors.
Use the following to run a large number of Map/Reduce jobs to
try to provoke a memory leak.

The following can be cut-and-paste'd into a Riak console/shell.
On my MBP, it runs for (very roughly) an hour.  Perhaps it should
run even longer to try to find very small memory leaks?
{ok, C} = riak:local_client().
[C:put(riak_object:new(<<"test">>, list_to_binary(integer_to_list(X)), list_to_binary(integer_to_list(X)))) || X <- lists:seq(1,500)].
[C:put(riak_object:new(<<"test">>, list_to_binary(integer_to_list(X)) ,<<"98123489123498123498123498123893489712348974123789432789234178942318794213897234178912348791234789412378923417894123789412378941237894123789412387943128794312879123478941237894123789412378941239781243789213487914237891423789142378914234">>)) || X <- lists:seq(1,250)].
io:format("Start time = ~p ~p\n", [time(), now()]), [ spawn(fun() -> {ok, C2} = riak:local_client(), [{ok,[_]} = C2:mapred_bucket(<<"test">>, [{map, {jsfun, <<"Riak.mapValuesJson">>}, none, false}, {reduce, {jsfun, <<"Riak.reduceSum">>}, none, true}]) || _ <- lists:seq(1,6*15)] , if YY == 1 -> os:cmd("say I am done now"), io:format("End time = ~p ~p\n", [time(), now()]); true -> ok end end) || YY <- lists:seq(1,200)].

These changes look good. Great catch, Scott! +1 to merge.


AARch64 SMP k_current_get performance improvements

Is your enhancement proposal related to a problem? Please describe.
Even after changes described in #32686, a best-case uncontended k_yield still takes ~826 cpu cycles. 49.8% of this time (!) is spent inside k_current_get or child calls thereof.
k_current_get is conceptually just a pointer lookup (return the address of the current thread). The current implementation is essentially:

(non-inlined) call to k_current_get (so the caller has to assume that this may trample a lot of registers).
(non-inlined) branch directly to z_impl_k_current_get (unnecessary and may cause e.g. an icache miss, but not the end of the world).
Lock out interrupts (inlined call to arch_irq_lock)
Grab the current core ID from mpidr_el1. (inlined call to arch_curr_cpu)
Calculate the base address of _kernel.cpus. (inlined call to arch_curr_cpu)
Do a multiply-add to find the address of _kernel.cpus[core & 0xFF]. (inlined call to arch_curr_cpu)
Load from offset 16 of the resulting pointer to calculate _current_cpu->current.
(non-inlined) call to arch_irq_unlock.

All told this takes ~28 cycles. (But there are 15 calls per k_yield, scattered through the callstack, which adds up very very quickly! Or about 417 cycles out of the 826 are just in calls to k_current_get.)
Note that the above numbers do not include second-order effects (such as surrounding code having worse register allocation due to having to do a call).
Describe the solution you'd like
Perhaps directly store _current in tpidr_el1, and &_kernel.cpus[core] in tpidrro_el0, or vice versa? This would reduce this from ~28 cycles to ~1, at the expense of having to update tpidr_el1 when you'd currently update _current.
A local proof-of-concept dropped uncontended k_yield from 826 cycles to 252 cycles (!).
(Function calls on AArch64 are surprisingly expensive. Most of the additional cycles saved there are just from avoiding stacking / juggling registers around / doing redundant work across function calls, so far as I can tell.)
After this optimization, what remains is:
70 cycles in z_impl_k_yield itself
69 cycles in z_swap_next_thread
50.5 cycles in z_swap_unlocked
14 cycles in z_is_t1_higher_prio_than_t2
11=5.5x2 cycles in should_queue_thread
9.5 cycles in sys_dlist_peek_head
5 cycles in update_cache
5=2.5x2 cycles in z_is_thread_state_set.constprop.0.isra.0
4.5 cycles in sys_dlist_is_empty
4.5 cycles in z_is_idle_thread_object.isra.0
3.5 cycles in z_is_thread_prevented_from_running.isra.0
2.5 cycles in is_aborting.isra.0
2=1x2 cycles in z_is_thread_queued.isra.0
1 cycle in k_yield
...and there are still some low-hanging fruit there (e.g. I included the long-tail function calls there because several of them really should be inlined for performance - e.g. sys_dlist_is_empty). (Ditto, there's two sets of spinlock lock/unlock per loop still - one in z_impl_k_yield, one in z_swap_unlocked.)
Describe alternatives you've considered
I looked at reducing calls to _current_cpu / _current - this is non-trivial, as the calls are largely scattered around and can't easily be combined.
I looked at storing _current in some manner that doesn't require an interrupt mask/unmask - this is again nontrivial. The best I could do was storing (sequence number, _current) atomicly with a retry loop, and it wasn't any faster than the current interrupt masking approach.
Additional context
Ties into #32686, #32187, https://github.com/zephyrproject-rtos/zephyr/pull/32557#discussion_r583292495.
ARM (both AArch32 and AArch64 - although I don't know what ARM version introduced this) has between three and five tpidr* registers, intended for storing 64 bits of thread-local data (thread ID, pointer, etc):

tpidr_el0. Normally accessible (read+write) from all ELs.
tpidrro_el0. Normally readable from all ELs and writable from EL1 and above.
tpidr_el1. Normally accessible (read+write) from EL1 and above.
tpidr_el2, if you have EL2. Normally accessible (read+write) from EL2 and above.
tpidr_el3, if you have EL3. Normally accessible (read+write) from EL3 only.

Currently AArch64 uses tpidr_el0 to store the TLS pointer, and otherwise does not use these registers. We can't rely on 4 or 5, but we can rely on 1-3 being available.
This proposal would have the following assignments:

tpidr_el0 is still the TLS pointer.
tpidrro_el0 stores &_kernel.cpus[core].
tpidr_el1 stores _current.

(Or swap 2/3. It doesn't matter too much.)

As an aside, take the cycle counts with a bit of a grain of salt. The total cycle count is accurate, but stalls can be delayed sometimes.
So e.g. function foo calls bar, function bar triggers a division and returns, foo tries to use the result of said division, the cycle count will sometimes show foo taking most of the cycles.

As an aside, as per usual I found an issue immediately after creating this... -fno-inline snuck into my local branch, which explains some of the inlining failures (although note that ALWAYS_INLINE overrides this).

That sounds perfect to me, though again I try to keep my nose out of ARM work.
It's worth noting that _current is just a field on struct _cpu, so as long as you have the first it's only an extra load from L1 away to get _current (that field is hit from everywhere, so you can assume it's cached if anything is).  Dedicating the extra register might actually hurt instead of help, depending on how the pipeline handles the special registers.  I know (or vaguely remember, anyway) that on x86 a lot of MSRs are actually slower than a read from L1 because of the way they're scheduled.
Note also that since this is an MMU-capable device, another option would be to keep a per-CPU page mapped somewhere (at the cost of a little extra work at context switch time) and read it from there, something like the way the per-thread VDSO works in linux.

@andyross -

as long as you have the first it's only an extra load from L1 away to get _current

Good point, but somewhat misleading.
The issue is, _current is a field in _current_cpu (logically speaking). The expensive part isn't reading _current. It's making sure that there isn't a race condition where you (as in thread T) read the current cpu, then you (as in thread T) get swapped to a different cpu, then you (as in thread T) read the old cpu's _current.
Right now the way that's avoided is via an irq lock / unlock, which isn't super expensive... until you want to do it 10x or more per context switch. And we currently do so 15x per context switch (at least on my local branch).
Directly reading _current from somewhere means that there is no race condition there because you're not going through _current_cpu, so you don't need the irq lock / unlock.

Dedicating the extra register might actually hurt instead of help, depending on how the pipeline handles the special registers

The Cortex-A53 (which is what I know) can't dual-issue a MRS/MSR, but otherwise is fairly good about reading system registers ("All system instructions are fully pipelined and most are single cycle in execution. System instructions that change processor configuration such as the SCTLR have 4 NOP cycles inserted behind to prevent a following instruction inheriting the old configuration settings."); this is definitely a concern for other A-series processors.

Note also that since this is an MMU-capable device, another option would be to keep a per-CPU page mapped somewhere (at the cost of a little extra work at context switch time) and read it from there, something like the way the per-thread VDSO works in linux.

You don't even need extra work at context switch, although it requires a memory map set up for it. Use TTBR1 for the per-cpu page and TTBR0 for everything else (or TTBR1 for the per-cpu page and all kernel data, and TTBR0 for everything else. Either works...). Set up all CPUs with the same TTBR0 and different TTBR1s.

@carlocaione - you may be interested in this (you seem to be interested in AArch64 improvements)

@andyross -

I try to keep my nose out of ARM work.

Fair enough. I had tagged you as "anything to do with context switching", and was mainly tagging you on these as you likely have a much better intuition for that side of things than I do. (E.g. if I'm spending a bunch of time trying to optimize something that isn't necessary in the first place.) On that note, a few questions:

Would it be possible/practical to avoid the multiple spinlock locks in context switching? Right now k_yield does lock/unlock/lock/unlock, which a) makes up a nontrivial portion of total context switch time, b) makes sched_lock even more contended, and c) in contended situations often results in additional latency when threads end up blocking on the second spin lock call.
Is it expected that there would be ~15 k_current_get calls in a context switch? I don't know if this is a case where AArch64's SMP implementation is slower than other architectures, my expectations as to how long a context switch should take on bare metal are skewed, etc, etc.

So, for 1: yes, in many/most cases that are a performance path (so... probably not k_yield() specifically, but in lots of places) we can do some work to roll locks together where it makes sense.  There is likely some code size and maintainability cost, but sure.  Remember though that spinlocks are intended to be super-fast in the uncontended case.  There should be 1-3 or so instructions issued in order (and a serialization cost on big CPUs) and you should exit the loop with a successful branch predict.  Even our naive implementation based on the atomics layer seems to get this mostly right on x86 and xtensa, at least (though x86 has a lot of newer toys we don't touch -- we're just a loop around LOCK CMPXCHG).
As for 2: I'm honestly surprised it's not more.  EVERYTHING is going to have something to say about the current thread.  Is maybe something in the ARM64 implementation defeating optimization?  What I would hope to see in the generated code is that the compiler would call it once and leave it in a register.  But if it's a function call without a __pure attribute or a volatile assembly block being inlined, then that won't work.

Hm... actually looking at it it doesn't work like I thought it did, in kernel_struct.h we have this when CONFIG_SMP:
#define _current k_current_get()

when what I assumed we had was something more like:
#define _current (arch_curr_cpu()->current)

Honestly that's just a bug.  It's going to force a function call, which is going to have to do the special register load every single time.  All for something we want in a register.

And thinking more carefully, it's actually wrong to make the evaluation of _current_cpu pure, I think.  Obviously threads can migrate!  We actually enforce right now that the macro is only ever expanded when interrupts are masked (for obvious correctness reasons).  But consider code like:
k = k_spin_lock(&lock);
printk("My CPU is %d\n", _current_cpu->id);
k_spin_unlock(&lock, k);

/* get preempted start running on a different CPU */

k = k_spin_lock(&lock);
printk("My CPU is %d\n", _current_cpu->id);
k_spin_unlock(&lock, k);

A compiler that tried to optimize the pure result of _current_cpu would print the same CPU ID twice, which is clearly wrong.  The spinlock calls have a memory clobber, but I don't think that's sufficient to defeat the optimization.
(Or maybe it is?  This gets to the difference between a "pure" and a "const" function that I always need to look up...)

So I think what we really want is a definition for current more like:
static ALWAYS_INLINE __attribute__(("pure")) __eval_current()
{
	return _current_cpu->current;
}

#define _current __eval_current()

But then we need to worry about toolchain portability and spend time staring at the generated assembly to be sure this is doing the right thing.

With inlining, after optimizations, it's 211 cycles / k_yield:
114 cycles in z_impl_k_yield
81 cycles in z_swap_next_thread
16 cycles in z_is_t1_higher_prio_than_t2

Or, expanding that out a little, for z_impl_k_yield
1.50   z_impl_k_yield:         stp	x29, lr, [sp,#-48]!
1.50   z_impl_k_yield+0x4:     mov	x29, sp
1.50   z_impl_k_yield+0x8:     stp	x19, x20, [sp,#16]
1.50   z_impl_k_yield+0xc:     stp	x21, x22, [sp,#32]
1.50   z_impl_k_yield+0x10:    mrs	x0, tpidr_el1
1.50   z_impl_k_yield+0x14:    ldrb	w0, [x0,#36]
1.50   z_impl_k_yield+0x18:    adrp	x6, .+81920 (0x10022138)
1.51   z_impl_k_yield+0x1c:    cbnz	w0, .+228 (z_impl_k_yield+0x100)
1.50   z_impl_k_yield+0x20:    mrs	x9, daif
1.50   z_impl_k_yield+0x24:    msr	daifset, #2
1.50   z_impl_k_yield+0x28:    movz	w0, #1
1.50   z_impl_k_yield+0x2c:    add	x2, x6, #0xc0c
1.50   z_impl_k_yield+0x30:    sevl
1.50   z_impl_k_yield+0x34:    wfe
1.50   z_impl_k_yield+0x38:    ldaxr	w1, [x2]
1.51   z_impl_k_yield+0x3c:    cbnz	w1, .-8 (z_impl_k_yield+0x34)
4.49   z_impl_k_yield+0x40:    stxr	w1, w0, [x2]
4.49   z_impl_k_yield+0x44:    cbnz	w1, .-16 (z_impl_k_yield+0x34)
2.34   z_impl_k_yield+0x48:    mrs	x0, tpidr_el1
2.34   z_impl_k_yield+0x4c:    ldrsb	w0, [x0,#25]
2.34   z_impl_k_yield+0x50:    tbz	w0, #31, .+36 (z_impl_k_yield+0x74)
1.71   z_impl_k_yield+0x74:    mrs	x5, tpidr_el1
1.71   z_impl_k_yield+0x78:    ldrb	w0, [x5,#25]
1.71   z_impl_k_yield+0x7c:    orr	w0, w0, #0xffffff80
1.71   z_impl_k_yield+0x80:    strb	w0, [x5,#25]
1.71   z_impl_k_yield+0x84:    mrs	x0, tpidr_el1
1.71   z_impl_k_yield+0x88:    cmp	x5, x0
1.72   z_impl_k_yield+0x8c:    b.eq	.+68 (z_impl_k_yield+0xd0)
1.66   z_impl_k_yield+0xd0:    mrs	x0, tpidr_el1
1.66   z_impl_k_yield+0xd4:    cmp	x5, x0
1.66   z_impl_k_yield+0xd8:    b.ne	.+16 (z_impl_k_yield+0xe8)
1.85   z_impl_k_yield+0xdc:    mrs	x0, tpidrro_el0
1.85   z_impl_k_yield+0xe0:    movz	w1, #1
1.85   z_impl_k_yield+0xe4:    strb	w1, [x0,#37]
1.85   z_impl_k_yield+0xe8:    mrs	x0, tpidrro_el0
1.85   z_impl_k_yield+0xec:    movz	w1, #1
1.85   z_impl_k_yield+0xf0:    strb	w1, [x0,#37]
1.85   z_impl_k_yield+0xf4:    add	x0, x6, #0xc0c
1.85   z_impl_k_yield+0xf8:    stlr	wzr, [x0]
1.85   z_impl_k_yield+0xfc:    msr	daif, x9
1.85   z_impl_k_yield+0x100:   mrs	x22, daif
1.85   z_impl_k_yield+0x104:   msr	daifset, #2
1.85   z_impl_k_yield+0x108:   mrs	x20, tpidr_el1
1.85   z_impl_k_yield+0x10c:   mrs	x0, daif
1.85   z_impl_k_yield+0x110:   msr	daifset, #2
1.85   z_impl_k_yield+0x114:   movz	w0, #1
1.85   z_impl_k_yield+0x118:   add	x21, x6, #0xc0c
1.85   z_impl_k_yield+0x11c:   sevl
1.85   z_impl_k_yield+0x120:   wfe
1.85   z_impl_k_yield+0x124:   ldaxr	w1, [x21]
1.86   z_impl_k_yield+0x128:   cbnz	w1, .-8 (z_impl_k_yield+0x120)
3.92   z_impl_k_yield+0x12c:   stxr	w1, w0, [x21]
3.92   z_impl_k_yield+0x130:   cbnz	w1, .-16 (z_impl_k_yield+0x120)
2.21   z_impl_k_yield+0x134:   bl	.-2728 (z_swap_next_thread)
0.797   z_impl_k_yield+0x138:   mov	x19, x0
0.797   z_impl_k_yield+0x13c:   cmp	x20, x0
0.797   z_impl_k_yield+0x140:   b.eq	.+140 (z_impl_k_yield+0x1cc)
2.72   z_impl_k_yield+0x190:   msr	daif, x22
2.72   z_impl_k_yield+0x194:   mrs	x0, tpidr_el1
2.72   z_impl_k_yield+0x198:   ldp	x19, x20, [sp,#16]
2.72   z_impl_k_yield+0x19c:   ldp	x21, x22, [sp,#32]
2.72   z_impl_k_yield+0x1a0:   ldp	x29, lr, [sp],#48
2.72   z_impl_k_yield+0x1a4:   ret
0.797   z_impl_k_yield+0x1cc:   stlr	wzr, [x21]
0.797   z_impl_k_yield+0x1d0:   b	.-64 (z_impl_k_yield+0x190)

That's mean_cycle_count offset instruction (note that the trace sometimes spreads out cycle counts somewhat, so e.g. don't treat this as "the entire final set of stack operations and returns took exactly the same number of instructions")

We actually enforce right now that the macro is only ever expanded when interrupts are masked (for obvious correctness reasons).

Uh, this isn't actually the case.
We check that _current_cpu is expanded only when non-preemptive, but not _current.
See, it is fine, conceptually speaking, to read _current even in a racy manner across context switches - because pretty much by definition _current will remain pointing to the current thread after we come back from the context switch.
But right now the way we do that is by having _current be a field of _current_cpu, which isn't fine to read in a racy manner.

Right.  By definition _current will never change from the perspective of generated code.  That's what it means, really :)

There should be 1-3 or so instructions issued in order (and a serialization cost on big CPUs) and you should exit the loop with a successful branch predict. Even our naive implementation based on the atomics layer seems to get this mostly right on x86 and xtensa, at least (though x86 has a lot of newer toys we don't touch -- we're just a loop around LOCK CMPXCHG).

It's more about the expected time taken than strict # of instructions. A CISC will (almost) always take less instructions...

we're just a loop around LOCK CMPXCHG).

What about interrupt lock / unlock?

Remember though that spinlocks are intended to be super-fast in the uncontended case.

Sure; what's your definition of super-fast here? Do the times I measured in #32686 for the uncontended case match your expectations? (For conversion from cycles<-> time, assume ~1GHz)
And also: this is sched_lock, which has more than a little bit of contention...

Right. By definition _current will never change from the perspective of generated code. That's what it means, really :)

Yep, really _current is just a threadlocal. But unforutnately it means that doing something like your previous suggestion:
static ALWAYS_INLINE __attribute__(("pure")) __eval_current()
{
	return _current_cpu->current;
}

is fundamentally unsafe if we could preempt.

I went and looked it up, by the way.  In fact gcc has us covered.  "const" means a function with a return value that depends on nothing but its arguments, "pure" means a function with a return value that depends on nothing but the arguments and global program state.  So in fact we want "const" for _current, meaning it's OK to optimize that under all circumstances, and "pure" for _current_cpu, which allows the compiler to optimize it only to the next function call or memory clobber.  Which is exactly what we want.  So in principle something like this should go into thread_structs.h:
/* These values are performance critical and we want the compiler to
 * be able to optimize their results, even if they are defined in
 * architecture code that might otherwise preclude that (for example
 * as C function calls, volatile __asm__ blocks, etc...).
 */

/* This function is "pure", because it can only be optimized until the
 * next context switch, so function calls and memory clobbers required
 * by locking operations will force it to be re-evaluated
 */
static ALWAYS_INLINE __attribute__(("pure")) __eval_curr_cpu(void)
{
	return arch_curr_cpu();
}

/* The current thread is by definition unchanging from the perspective
 * of generated code, so this is "const" allowing the compiler to
 * cache its value in any manner.
 */
static ALWAYS_INLINE __attribute__(("const")) __eval_current(void)
{
	return __eval_curr_cpu()->current;
}

/* Note that _current_cpu can change as the thread migrates, so it's
 * never legal to use this result if preemption is possible.  The
 * assert is there to catch mistakes.
 */
#define _current_cpu ({ __ASSERT_NO_MSG(!z_smp_cpu_mobile()); \
			__eval_curr_cpu(); })

#define _current __eval_current()

I'll submit when I get a chance, but please try this out if you want and see if the generated code looks correct to you.

And regarding the preemption-safety of _current.  You're absolutely right that in principle _current should be usable in a preemptible context.  We just don't have any such code right now[1]: everywhere that touches it is in an ISR or under a lock.  And even if you could... there's not much in the thread struct to be inspected that doesn't require synchronization.
[1] Well, the assert only exists in SMP, so it's possible there are code paths on single CPU configurations that try this I guess.  But the point is moot there as _current_cpu == 0 by definition.

everywhere that touches it is in an ISR or under a lock.

I think I'm missing something. You've asserted this a few times, but I do not think this is the case:
See e.g. eswifi_lock:
static inline void eswifi_lock(struct eswifi_dev *eswifi) {
	if (atomic_get(&eswifi->mutex_owner) != (atomic_t)(uintptr_t)_current) {
	[...]
}

Or k_mbox_put:
int k_mbox_put(struct k_mbox *mbox, struct k_mbox_msg *tx_msg, k_timeout_t timeout) {
	tx_msg->_syncing_thread = _current;
	[...]

Neither of these are required to be in locked/ISR context I believe.

please try this out if you want and see if the generated code looks correct to you.

Sure, shall do.
I am mildly concerned here about the scope of const/pure. I could see a compiler writer (note: I don't think either gcc or clang do this currently) wrapping __eval_current with, roughly:
thread_t *wrap___eval_current(void) {
    static thread_t *wrapped = __eval_current(); /* assume threadsafe static initialization */
    return wrapped;
}

...which is fine according to the definition, but would rather fall apart here.

This proposal would have the following assignments:
1. `tpidr_el0` is still the TLS pointer.
2. `tpidrro_el0` stores `&_kernel.cpus[core]`.
3. `tpidr_el1` stores `_current`.

Just an heads up that tpidrro_el0 soon will be not available anymore because we are going to use that to implement arch_is_user_context() as part of the userspace implementation that we are going to push next week.

Wait, how are you using it for arch_is_user_context? Is there a PR for this?

Wait, how are you using it for arch_is_user_context? Is there a PR for this?

There will be soon, hopefully today or tomorrow, for sure by the end of this week.

There will be soon, hopefully today or tomorrow, for sure by the end of this week.

Great. I'm hoping that the use for arch_is_user_context can be coordinated with this. It's a major performance win.

Unfortunately, due to #32710 the above measurements are incorrect. Or rather, they were measuring something different than I had expected.
I was trying to measure k_yield as a quick (as in low-overhead) way to measure context switch time.
Unfortunately, due to #32710 k_yield wasn't actually yielding, and it took me an embarrassingly long time to realize that that was the case.
So this was measuring/optimizing the time for k_yield when it didn't context-switch.

Wait, how are you using it for arch_is_user_context? Is there a PR for this?

@jharris-intel well, no reason to wait.
Anyway the gist is that we use tpidrro_el0 as a register to hold a value to indicate whether we are in user or kernel context. Given that we do not have access to currentEL from EL0 this was an handy option to quickly determine that.

Anyway the gist is that we use tpidrro_el0 as a register to hold a value to indicate whether we are in user or kernel context. Given that we do not have access to currentEL from EL0 this was an handy option to quickly determine that.

Hmm. So you're setting it / clearing it on every kernel-mode entry/exit then?
What's the advantage over putting this as a member of the cpus struct?
One possibility...
1. `tpidr_el0` is still the TLS pointer.
3. `tpidrro_el0` stores `_current` if in EL1, NULL if in usermode.
2. `tpidr_el1` stores `&_kernel.cpus[core]`.

(As you can't access _current in usermode anyway I believe, and on a context switch you know what you're switching to to restore _current...)
Then your "is in kernel context" check becomes tpidrro_el0 != NULL.

(You could also do something fancy like take advantage of the alignment of cpu and set/clear the low bit; I don't know if it's worth it.)


⚠ ClamAV has degraded performance

In 8b954ca, ClamAV (https://spamassassin.apache.org/) experienced degraded performance:

HTTP code: 200
Response time: 73 ms

Resolved: ClamAV performance has improved in fc9d462.


How to view previous image - When i Tapped Random Image [x] in a list

Hi,
Currently i am using photomanger plugin to display the image.
Everything is working fine. If i am tapping the image from first [0]
Example: if am having List = [0,1,2,3,4,5,6];
If i am tapping the image from 0th index - Everything is working fine i can able to go next / go previous.
But the problem is if i am tapping the image from [3]rd index. i can able to view Next image but not previous image.
For Example : I can able to swipe next from [4,5,6...] But not able to swipe left from [3,2,1,0..]
Please tell me. How to achieve this.
Sharing my code:
import 'dart:io';
import 'package:extended_image/extended_image.dart';
import 'package:flutter/material.dart';
import 'package:photo_manager/photo_manager.dart';

class SimplePhotoViewDemo extends StatefulWidget {
  List<AssetEntity> newList;
  int indexofKey;

  SimplePhotoViewDemo(
      {Key? key, required this.newList, required this.indexofKey})
      : super(key: key);

  @override
  _SimplePhotoViewDemoState createState() => _SimplePhotoViewDemoState();
}

class _SimplePhotoViewDemoState extends State<SimplePhotoViewDemo> {
  int counter = 0;
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('ExtendedImageGesturePageView'),
      ),
      body: ExtendedImageGesturePageView.builder(
        controller: ExtendedPageController(
          initialPage: 0,
          pageSpacing: 50,
        ),
        itemCount: widget.newList.length,
        onPageChanged: (int index) {
          counter = widget.indexofKey + index;
          setState(() {
            counter = counter;
          });
        },
        itemBuilder: (BuildContext context, int index) {
          return ExtendedImage(
            image: AssetEntityImageProvider(counter == 0
                ? widget.newList[widget.indexofKey]
                : widget.newList[counter]),
            fit: BoxFit.contain,
            mode: ExtendedImageMode.gesture,
            initGestureConfigHandler: (ExtendedImageState state) {
              return GestureConfig(
                //you must set inPageView true if you want to use ExtendedImageGesturePageView
                inPageView: true,
                initialScale: 1.0,
                maxScale: 5.0,
                animationMaxScale: 6.0,
                initialAlignment: InitialAlignment.center,
              );
            },
          );
        },
      ),
    );
  }
}

Also sharing the video file for reference.
Below Video - Is working fine as expected
https://user-images.githubusercontent.com/45402258/195795192-6869f299-f418-4a21-9e7d-7c17a63a2f2a.MP4
Below Video - Not working fine as expected.
https://user-images.githubusercontent.com/45402258/195795389-a4a963f4-b72e-4f47-8bcb-e11698d102f0.MP4
Thank you.

package version

extended_image: ^6.3.1
photo_manager: ^2.3.0

Any update on this?

Hi,
Ive fixed this issue by passing the the index in initalPage parameter
   controller: ExtendedPageController(
          initialPage: widget.indexofKey, //Tapped image index value.

        ),

Thank you.


Unknown section checks in app config file

I have a simple config file like this:
name: app
type: "python:3.7"

dependecies:
    python:
       pipenv: '*'

hooks:
    build: |
      pipenv install --system

disk: 1024

web:
    commands:
        start: python server.py

CLI currently checks if the required fields (name, type and disk) are present. In case there is some extra non existing section like dependecies with missing n letter it is not detected and the build simply fails during the build hook. Similar typos and mistakes are easy to make, but hard to debug. Could be some additional logic added to prevent issues like this one?

This should be improved somewhere, but I'm not sure how. The problem is currently that Platform.sh's (git) server-side validation of the .platform.app.yaml file deliberately does allow any root keys in that file. I think it's partly for forwards and backwards compatibility, and partly (I imagine?) so that custom YAML syntax can be supported.
The CLI shouldn't include much more complicated schema/validation logic than the server (maintenance and support of older versions would get out of control), but maybe there's a way to subtly warn about typos like this.

The strict validation is maybe not necessary. I can imagine something like a warning
'dependecies' is not a known entry, did you mean 'dependencies'?
in case the entry that is really similar to the existing one is detected. User can ignore it in case he knows what is he doing and it will still serve its purpose.


pythonPackage.colorama: 0.3.3 -> 0.3.6

Motivation for this change
0.3.6 is required by git-up.
Things done

[X] Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)

[X] NixOS

[X] Fits CONTRIBUTING.md.

Travis fails because there are dependencies that will need 0.3.3 (awscli).

Hi @FRidh, saw that, thanks.
What is the proper way to do this? Make a special v0.3.3 for awscli or a special 0.3.6 for git-up?

@peterhoeg check how it is done with django. We have multiple versions, and an attribute pointing to the default, which is preferably the latest. We will have to check if everything else still functions after this upgrade.

closed with bf06024b9f68086ff2c3293ad90473141c0a96c2 and 778712e0a778faec827cf593737ff760d0e6b08b


github container-publish: Add qemu action + update docker action version

Here's how to check the hash for a github action:
gh repo clone docker/setup-buildx-action
cd setup-buildx-action
git tag -l --sort=-v:refname  # List all tags
$ git rev-parse v2.10.0
885d1462b80bc1c1c7f0b00334ad271f09369c55
$ git rev-parse v3
988b5a0280414f521da01fcc63a27aeeb4b104db

Or got to repo on the github website (releases -> tags)
https://github.com/docker/setup-buildx-action/releases/tag/v2.10.0
885d1462b80bc1c1c7f0b00334ad271f09369c55
https://github.com/docker/setup-buildx-action/releases/tag/v3.6.1
988b5a0280414f521da01fcc63a27aeeb4b104db

I updated the PR to use pinned hashes. Also added  a PR description. Can someone else please review?


Replace strcmp() with strncmp()

This is for #731. Arguably in some of the cases we don't really need *n* version (string literals) or just trade one potential segmentation fault (from unbound strcmp()) to another (from unbound strlen() - strnlen() would probably be an answer here, but I'm not sure we want to go down that path).
But even if the above holds, it still makes sense from the code unification standpoint - we use strncmp() everywhere else.
Side note - I've noticed in most of the cases we use strncmp(), we are actually doing a substring comparisons (i.e. without a null-terminator taken into account). While this apparently works ok - as no one complained so far and nothing broke - I could go over and replace that with exact match checks. Let me know what you think about that.

Uh. When would strncmp(a, b, strlen(b)+1) avoid a problem that strcmp(a, b) would have? Can you give an example?
Hint: never. ;-)
Sorry to have to point out the obvious: strcmp will compare at most MIN(strlen(a)+1, strlen(b)+1) characters, and your strncmp at most strlen(b)+1
The "problem" with strcmp occur when both input strings are not (properly) terminated. And using strlen will not help you there.
A probably better way would be to have a #define that limits the maximum size of names for devices, and using that in strncmp. While cludgey, using any upper limit is still better than unbounded.

Thanks for your comments. I know all of that and considered all points you have highlighted, when preparing the PR - thus my comment to it and stressing the "code unification" above everything else
Additionally, for sizeof() vs strlen() specifically:

Compiler optimization replaces strlen() with a const anyway, when possible;
Sizeof will not properly work on pointers and may confuse relatively inexperienced developers, so this is for clarity and readability.

Good shout on the fact we dont actually check terminating char. That could bite us as gpio-1 would match gpio-11 etc... Probably worth fixing at some point.

Wonder how Linus Torvalds would describe this ... ;-)

If you provide a better fix then you're welcome to submit it ;-) In the meantime if Linus is available for a code review, we'd obviously love that input.

Changing things like:
if (strcmp(argv[1], "help") == 0) {
to
if (strncmp(argv[1], "help", strlen("help") + 1) == 0) {
is where I imagined Linus using some ... colourful ... langauge about it.
For constant strings like here, it does not make any sense at all. It does not make the code any better, any more efficient, any easier to read or write. Rather the opposite in all possible ways.
The only way I would touch that line would be to make it
if (!strcmp(argv[1], "help")) {
but that would going in the opposite direction. But hey, that's me, and I don't really feel it is my place to object.

Good shout on the fact we dont actually check terminating char. That could bite us as gpio-1 would match gpio-11 etc... Probably worth fixing at some point.

Submitted #736 to track this.

Actually, the new version with strncmp is strictly worse than the old with strcmp from a program robustness point-of-view as well.
strncmp(a, b, strlen(b)) will go ballistic if one of the strings (here b) is badly terminated. strcmp(a, b) needs both to be badly terminated.
So for instance if a is "\0" (or any string shorter than the allocation for b) and b is unterminated (longer than a), strcmp will not crash while the strncmp version will. There is no case where strcmp will crash but strncmp will not.

You make this statement based on what exactly? Looking at str{n}cmp() sources in Linux, I see they use a null-terminator from the first string as the comparison loop exit marker, in both functions. And in strncmp() there's that n limit on top, which limits the number of cycles - and which in our code is based on the second string, so to me it looks like in our case we actually are a bit more protected, by having a limit based on both strings.
I haven't analyzed the arch-specific implementations, my assumption being that they're algorithmically equivalent.

@alext-mkrs : Ok let's go back to school :-)
(Discussion about the point you raised after the proof)
"Theorem":
There are cases of a and b where strncmp(a, b, strlen(b)) can fail in when strcmp(a, b)  cannot.
There are no cases where strcmp(a, b) can fail but strncmp(a, b, strlen(b)) cannot.
Proof:
Let OK denote no problem, and NG mean that there is chance for invalid memory access. Also let "terminated" mean that there is at least one null byte within the allocated block of a string.
Compare strcmp(a, b) vs strncmp(a, b, strlen(b)) and divide it into six cases based on the termination statuses of a and b as follows.
i) a terminated, b terminated  => strcmp OK, strncmp OK
ii) a unterminated, b terminated, b shorter (or equal) than a's allocation => strcmp OK, strncmp OK
iii) a unterminated, b terminated, b longer than a's allocation => strcmp NG, strncmp NG
iv) a terminated, b unterminated, a shorter (or equal) than b's allocation => strcmp OK, strncmp NG
v) a terminated, b unterminated, a longer than b's allocation => strcmp NG, strncmp NG
vi) a unterminated, b unterminated => strcmp NG, strncmp NG
In case iv) strncmp can fail, but strcmp cannot. This proves the first statement. There are no cases above where strcmp fails but strncmp cannot. This proves the second statement.
QED
Regarding the argument about which null byte implementations use internally.
It does not matter whether str{n}cmp function looks at the null byte of the first, second or both arguments. The comparison stops at the first null byte, regardless of which string it occurs in. If one string has a null char at position k and the other not does not have a null char at position k, the strings differ at positon k and the functions return. If both have a null byte, checking any one of them is enough.
With the strncmp way, the problem is moved to strlen. It can always fail for unterminated strings.
My thoughts of using strncmp instad of strcmp only changes to the worse in pretty much every way I can imagine:

Code is longer
Code is harder to read
Code is harder to write
Code is more inefficient
Code is uglier (this is subjective)
Code is less general
Code is harder to maintain

So what is the benefit? How did this ... unusual ... idea even come by without anyone reacting?
While I don't feel this is my code to maintain, and there are strong emotions to use strncmp ...  go ahead. Use the gun. Aim at your foor. But sane, it is not. :-)

@TechNexion If your argument is that strcmp -> strncmp is these cases has little value I agree. And actually I've learnt quite abit about strcmp so that's been interesting :)
However I'm really not sure this PR it merits the - whilst passionate - long debate above. If you feel really strongly about it, fix all the strncmp that are not required and add a quick bit of doc that explains why strcmp is the appropriate choice so the next guy with static analysis doesn't take the easy path. I'll happily merge that PR :)

@technexion, thanks for analysis, that formally proves my description of this PR, where I'm saying that we're trading one potential segfault for the other ;-) and that th main goal of the PR is code unification.
Now getting back to why we need this whole thing at all - to protect us from unterminated strings - the solution is really just to replace the strlen(string_of_unknown_length) with some hard cap a'la MAX_SIZE we use elsewhere.
Oh and yes, the whole topic is a bit moot as all of us know and understand what's going on, it's just the choice we made based on factors beyond strictly technical area.


Add documentation for each function in readme

https://www.npmjs.com/package/jsdoc
What about using jsdoc for documenting?
I wanted to give it a try

Yes, I let you set up it.


Show cards used to build wonder

I know it's not a real feature of the game, but at the end, could we see the cards used to build the phases of each wonder? (For analysis, to see who hate-drafted that one card I needed.)

Haha definitely a feature worth looking into. I'll check how hard it is to add. Like maybe on hover we could know.
As far as I remember I tried not to send any data about hidden things to the clients, so they can't cheat. But maybe we can change this at the end of the game.
Thanks for the suggestion!


How can I get a already solved issue be merged faster to the stable release?

Please see issue:
Right shift and digit 4 press bug on Windows Desktop
#110852
This issue is solved in flutter 3.4 for windows, but still not active, why can this fix not be released sooner? Maybe someone can add this merge to the next stable release or increase priority?
I think this is a serious bug for people building a windows application, backspace after the bug does not work anymore in a text field.
The commit that fixed the issue:
https://github.com/flutter/engine/blob/flutter-3.4-candidate.0/shell/platform/windows/keyboard_key_embedder_handler.cc

Hello @Spitskool42. You can read about the Cherrypick process and request one using the template provided under Requesting a cherry-pick. If you want to know about the prioritization process of issues, please see Issue hygiene.
Closing this issue as this is not a bug or feature request. Thank you.


viewMessages and secret chat do not work

I'm trying to get marked messages as read in a secret chat:
{"@type":"viewMessages","chat_id":-1998211077445,"force_read":1,"message_thread_id":0} {"@type":"ok"}
But messages do not turn marked as read.
I used to try openChat and then viewMessages, but no result. What I do wrong?

You didn't specified identifiers of messages, which was viewed.

What Function I have to use so be able to get identifiers of all messages in the chat?

If you want to read all messages, you can pass identifier of the chat.last_message message.


[main] Update actions

Cron -knative-prow-robot
/cc knative-sandbox/kn-plugin-func-approvers
/assign knative-sandbox/kn-plugin-func-approvers
Produced by: knative-sandbox/knobots/actions/update-actions
Details:

Well this was... not expected.
Explicitly placing this on hold while we determine 1) whether this is needed in the templates and 2) if so how to do it properly.
/hold

/rebase

/unhold

/lgtm

/approve
/me still needs to learn all of these commands better


Training freezed at last batch of first epoch , not starting validation generator

logs.txt
Log file attached
Code
#!/usr/bin/env python
# coding: utf-8

# In[1]:

from PIL import Image
import os
import cv2
import numpy as np
import time
from scipy import ndimage
import random
from model import Deeplabv3
import tensorflow as tf
# import numpy as np
# sess = tf.compat.v1.Session(config=tf.compat.v1.ConfigProto(log_device_placement=True))
import math
from tensorflow.python.keras.applications.imagenet_utils import preprocess_input
from keras.utils import to_categorical
from keras.callbacks import ModelCheckpoint, EarlyStopping
from tensorflow.keras.callbacks import TensorBoard
import matplotlib.pyplot as plt

# In[2]:

pd_dir='../../../ssd_scratch/cvit/ritvik_agr/Combined_Dataset/'
#path to provided foreground images
fg_path_1 = pd_dir+'Training_set/Adobe-licensed images/fg/'
#path to provided alpha mattes
a_path_1 = pd_dir+'Training_set/Adobe-licensed images/alpha/'
#path to provided foreground images
fg_path_2 = pd_dir+'Training_set/Other/fg/'
#path to provided alpha mattes
a_path_2 = pd_dir+'Training_set/Other/alpha/'
bg_path_train = pd_dir+'train2014/'
bg_path_val=pd_dir+'VOCtrainval_11-May-2012/VOCdevkit/VOC2012/JPEGImages/'
val_path_a=pd_dir+'Test_set/Adobe-licensed_images/alpha/'
val_path_fg=pd_dir+'Test_set/Adobe-licensed_images/fg/'
batch_size_user=20
input_image_shape=512
#Path to folder where you want the composited images to go
# out_path = 'merged/'

# In[3]:

# def composite4(fg, bg, a, w, h):
#     bbox = fg.getbbox()
#     bg = bg.crop((0,0,w,h))
#     fg_list = fg.load()
#     bg_list = bg.load()
#     a_list = a.load()
#     for y in range(h):
#         for x in range (w):
#             alpha = a_list[x,y] / 255
#             t = fg_list[x,y][0]
#             t2 = bg_list[x,y][0]
#             if alpha >= 1:
#                 r = int(fg_list[x,y][0])
#                 g = int(fg_list[x,y][1])
#                 b = int(fg_list[x,y][2])
#                 bg_list[x,y] = (r, g, b, 255)
#             elif alpha > 0:
#                 r = int(alpha * fg_list[x,y][0] + (1-alpha) * bg_list[x,y][0])
#                 g = int(alpha * fg_list[x,y][1] + (1-alpha) * bg_list[x,y][1])
#                 b = int(alpha * fg_list[x,y][2] + (1-alpha) * bg_list[x,y][2])
#                 bg_list[x,y] = (r, g, b, 255)
#     return bg

# In[4]:

def generate_trimap(alpha):
   k_size = random.choice(range(2, 5))
   iterations = np.random.randint(5, 15)
   kernel = cv2.getStructuringElement(cv2.MORPH_ELLIPSE, (k_size, k_size))
   dilated = cv2.dilate(alpha, kernel, iterations=iterations)
   eroded = cv2.erode(alpha, kernel, iterations=iterations)
   trimap = 128*np.ones(alpha.shape, dtype=np.uint8)
   trimap[eroded >= 255] = 255
   trimap[dilated <= 0] = 0
   return trimap

# In[5]:

# def composited_image(im_name,bg_name):
#     if os.path.isfile(fg_path_1+im_name):
#         im = Image.open(fg_path_1 + im_name);
#         a = Image.open(a_path_1 + im_name);
#     elif os.path.isfile(fg_path_2+im_name):
#         im = Image.open(fg_path_2 + im_name);
#         a = Image.open(a_path_2 + im_name);
#     else:
#         im = Image.open(val_path_fg + im_name);
#         a = Image.open(val_path_a + im_name);

#     bbox = im.size
#     w = bbox[0]
#     h = bbox[1]

#     if im.mode != 'RGB' and im.mode != 'RGBA':
#         im = im.convert('RGB')

#     if os.path.isfile(bg_path_train+bg_name):
#         bg = Image.open(bg_path_train + bg_name);
#     else:
#         bg=Image.open(bg_path_val+bg_name)

#     if bg.mode != 'RGB':
#         bg = bg.convert('RGB')

#     bg_bbox = bg.size
#     bw = bg_bbox[0]
#     bh = bg_bbox[1]
#     wratio = w / bw
#     hratio = h / bh
#     ratio = wratio if wratio > hratio else hratio
#     if ratio > 1:
#         bg = bg.resize((math.ceil(bw*ratio),math.ceil(bh*ratio)), Image.BICUBIC)
#     out = composite4(im, bg, a, w, h)
#     return cv2.resize(np.array(out),(512,512)),cv2.resize(generate_trimap(np.array(a)),(512,512))

# In[6]:

def combined_img(im_name,bg_name):
    if os.path.isfile(fg_path_1+im_name):
        foreground = Image.open(fg_path_1 + im_name);
        alpha = cv2.imread(a_path_1 + im_name);
    elif os.path.isfile(fg_path_2+im_name):
        foreground = Image.open(fg_path_2 + im_name);
        alpha = cv2.imread(a_path_2 + im_name);
    else:
        foreground = Image.open(val_path_fg + im_name);
        alpha = cv2.imread(val_path_a + im_name);

    if foreground.mode != 'RGB':
        foreground = foreground.convert('RGB')

    if os.path.isfile(bg_path_train+bg_name):
        background = Image.open(bg_path_train + bg_name);
    else:
        background=Image.open(bg_path_val+bg_name)

    if background.mode != 'RGB':
        background = background.convert('RGB')

    foreground = np.array(foreground)
    background = np.array(background)

    background=cv2.resize(background,dsize=(foreground.shape[1],foreground.shape[0]))
    # Convert uint8 to float
    foreground = foreground.astype(float)
    background = background.astype(float)

    # Normalize the alpha mask to keep intensity between 0 and 1
    alpha = alpha.astype(float)/255

    # Multiply the foreground with the alpha matte
    foreground = cv2.multiply(alpha, foreground)

    # Multiply the background with ( 1 - alpha )
    background = cv2.multiply(1.0 - alpha, background)

    # Add the masked foreground and background.
    outImage = cv2.add(foreground, background)
    return cv2.resize(np.array(outImage),(512,512)),cv2.resize(generate_trimap(np.array(255*alpha)),(512,512))[:,:,0]

# In[7]:

def train_gen():
    with open(pd_dir+'Training_set/training_fg_names.txt') as f:
        fg_files = np.array(f.read().splitlines()).reshape((-1,1))
    with open(pd_dir+'Training_set/training_bg_names.txt') as f:
        bg_files = np.array(f.read().splitlines()).reshape((len(fg_files),-1))

    n=0
    x_img=[]
    y_img=[]
    while True:
        i=np.random.randint(0,431)
        j=np.random.randint(0,100)
        while bg_files[i,j]=='-1':
            i=np.random.randint(0,431)
            j=np.random.randint(0,100)
        x,y=combined_img(fg_files[i,0],bg_files[i,j])
        x_img.append(x)
        y=to_categorical(y//127,num_classes=3)
        y_img.append(y)
        bg_files[i,j]=-1
        n+=1
        if n==batch_size_user:
            yield preprocess_input(np.array(x_img),mode='tf'),np.array(y_img)
            n=0
            x_img=[]
            y_img=[]

# In[8]:

def val_gen():
    with open(pd_dir+'Test_set/test_fg_names.txt') as f:
        val_fg_files = np.array(f.read().splitlines()).reshape((-1,1))
    with open(pd_dir+'Test_set/test_bg_names.txt') as f:
        val_bg_files = np.array(f.read().splitlines()).reshape((len(val_fg_files),-1))
    n=0
    x_img=[]
    y_img=[]
    while True:
        i=np.random.randint(0,50)
        j=np.random.randint(0,20)
        while val_bg_files[i,j]=='-1':
            i=np.random.randint(0,50)
            j=np.random.randint(0,20)
        x,y=combined_img(val_fg_files[i,0],val_bg_files[i,j])
        x_img.append(x)
        y=to_categorical(y//127,num_classes=3)
        y_img.append(y)
        val_bg_files[i,j]=-1
        n+=1
        if n==batch_size_user:
            yield preprocess_input(np.array(x_img),mode='tf'),np.array(y_img)
            n=0
            x_img=[]
            y_img=[]

# In[ ]:

# In[9]:

model_1=Deeplabv3(backbone='xception')
model=Deeplabv3(backbone='xception',classes=3)
for l_tg,l_sr in zip(model.layers[:-2],model_1.layers[:-2]):
        l_tg.set_weights(l_sr.get_weights())
del model_1
for l in model.layers[:356]:
    l.trainable=False
checkpoint = ModelCheckpoint("./best_model.h5", monitor='val_loss', verbose = 1, save_best_only=True, mode = 'auto')
earlystop =  EarlyStopping(monitor='val_loss', min_delta=0.01, patience=5, verbose =1, mode="auto", restore_best_weights=True)
tb_callback=TensorBoard(log_dir="logs_tb/{}".format(int(time.time())),histogram_freq=1)
model.summary()

# In[11]:

model.compile(optimizer='adam', loss=tf.keras.losses.CategoricalCrossentropy(from_logits=True))

# In[14]:

model.fit(train_gen(),steps_per_epoch=43100//batch_size_user,validation_data=val_gen(),validation_steps=1000//batch_size_user,epochs=100,callbacks=[checkpoint, earlystop,tb_callback])

# In[ ]:

@ritvikagrawal1 Can you please share a simple standalone code to reproduce the issue? I tried your code in the gist. It throws some errors. Can you please make it standalone (adding cloning model repo, etc).
Can you also mention what TF version you have used and add any more details that you think will help us resolve this faster. Thanks!

@jvishnuvardhan
please clone this repo: https://github.com/bonlime/keras-deeplab-v3-plus
and additionally, add the code in a new file I mentioned above.

in case if dataset is not accessible, you can try on any custom images
i/p size - 512,512,3
o/p size - 512,512,3 (one-hot)

in order to make code run, you can consider any i/p of np.ones((512,512,3)) as both input-output @jvishnuvardhan

@ritvikagrawal1 It would really helpful if you provide a SIMPLE standalone code to reproduce the issue. It will also help minimizing back and forth communications and reach a resolution. Generally, we would ask the users to post any support related questions (with long code) to post them in Stackoverflow where SO team will resolve those issues. Thanks!

@ritvikagrawal1
I don't think the training froze in the first place. Note that one batch of your data took 15 seconds to train. The validation data consists of 100 batches, which took 25 minutes to train on the last batch, looking at your logs. Thus, the validation of 100 batches will take about the same time, as the model is extremely complex. This may lead you into thinking that the training/validation has froze.
I highly recommend using TensorFlow's in-built image preprocessing methods and using tf.data and @tf.function decorator. It will make the pipeline much faster, if used correctly.
Hope this helps.

@ritvikagrawal1 from your log, it looks like  you wanted to use GPU but it was not detecting GPU because some of the GPU related libraries are missing. As your model is complex and was running on cpu, it was taking longer time. May be you can open a new issue related to build/install of TF. Thanks!


Do the parameters not support Like queries?

Thank you very much for creating this library, it works very well, but I recently encountered a problem with fuzzy queries.
This is my code, sql statement tested is available.
    pub fn find_word_by_regexp_keyword(
        &self,
        keyword_reg: &str,
    ) -> Result<Vec<Word>, rusqlite::Error> {
        let mut stmt = self
            .connection
            .prepare(r#"SELECT keyword FROM word WHERE keyword LIKE ":keyword_reg%";"#)?;
        let words = stmt
            .query_map::<Word, _, _>(&[(":keyword_reg", &keyword_reg)], |row| {
                self.find_word_by_keyword(&row.get::<_, String>(0).unwrap())
            })?
            .collect::<Result<Vec<Word>, _>>()?;
        Ok(words)
    }

With the above code, I get an error:
Err: InvalidParameterName(
    ":keyword_reg",
)

My sqlite version is : 3.37.0 2021-11-27 14:13:22

This is not related to rusqlite.
You should be able to reproduce your issue with SQLite shell (see .parameter here).
And not related but you should use single quote for string literal.
For me, you need to do:
SELECT keyword FROM word WHERE keyword LIKE (:keyword_reg || '%');

See #573 for a similar issue.


Change tabIndex to positive for keyboard tab order [#68]

To verify tab key should skip checkboxes

Coverage remained the same at 92.823% when pulling 0e3d4bcdbd0da07c2ea1ac19a13180897fb808f5 on tabindex into 00bc2230b1ed9ccea150f98caf81d7297a286051 on master.

Coverage remained the same at 92.823% when pulling 0e3d4bcdbd0da07c2ea1ac19a13180897fb808f5 on tabindex into 00bc2230b1ed9ccea150f98caf81d7297a286051 on master.

Coverage remained the same at 92.823% when pulling 1c45f8707665844bff905e45bb61740f3b4080f3 on tabindex into 00bc2230b1ed9ccea150f98caf81d7297a286051 on master.

Coverage remained the same at 92.823% when pulling 1c45f8707665844bff905e45bb61740f3b4080f3 on tabindex into 00bc2230b1ed9ccea150f98caf81d7297a286051 on master.