Randomized Algorithms for Optimizing Large Join Queries

I've edited a bit in the base class, which I want to use for all three algorithms.
Because of this, I've decided to create this is as my common branch. To make things easier for you to know what is what and keep order, I will create branches for each of the algorithms out of this branch. I'm also going to start working on two-phase optimization (2PO) soon, and I'll likely look back and forth a lot between all of the algorithms as I want to possibly have the Iterative and Annealing algorithms have an optimize function which is static and ideal for use in the 2PO in a way that the 2PO becomes sufficiently usable with different settings (input values to the 2PO will include things such as how many generations II will run before SA takes over, etc).

That makes it very hard for me to see what you have done here (at least if I look only in the Github Web interface, which is where I have to look to put review comments).

Gotcha. This branch contains all changes to SA, as well as the introduction of a generic function to the base class which I use in the base class.
The SA class is almost but not quite done. There's something in the paper which I simply do not understand, which I need to understand before I can finish the algorithm. The algorithm paper implies that 16*(number of joins in query) is a boolean and I don't understand what this entails.

Simulated Annealing almost done, but I am at a loss for how to implement Equilibrium. It's supposed to be a boolean from what I can read in the paper, but the paper's example implementation defines it to be 16*(number or joins in query). (table 2.2, page 314 in the paper, page 3 in the pdf)

but I am at a loss for how to implement Equilibrium. It's supposed to be a boolean from what I can read in the paper, but the paper's example implementation defines it to be 16*(number or joins in query). (table 2.2, page 314 in the paper, page 3 in the pdf)

I agree that this aspect of the algorithm should have been stated more explicitly in the paper. However, my interpretation is as follows: The equilibrium is assumed to be reached after the inner loop has been run a fixed number of times, where this number is 16 * (the number of joins in the query). Hence, for queries with more joins, the inner loop runs more often.
The follow-up question then is: What do we consider as "the number of joins in the query"? The issue is that the paper considers only join plans but, in our case, there may also be other operators in the plans (i.e., not only joins). A possible solution would be to adapt the equilibrium condition to our setting as follows. Instead of fixing the number of inner-loop runs to be 16 * (the number of joins in the query), we fix it to be 16 * (the number of logical operators in the given logical plan). What do you think of this idea?

Instead of fixing the number of inner-loop runs to be 16 * (the number of joins in the query), we fix it to be 16 * (the number of logical operators in the given logical plan). What do you think of this idea?

This seems sound, at least for now when we do not have any other idea. Testing later on will provide the data we need to evaluate the performance of all of our settings.
I'm thinking of whether I should be implementing an equilibrium condition for the SA algorithm, similar to how the II algorithm has a stopping condition, and also how I should best pass the number of operators to the optimize method, as the optimize method itself only deals with the physical plan and not the logical plan. I don't know what difference it would make, but we could also count operations in the physical plan instead of counting them in the logical plan.
Regardless, I've looked into LogicalPlanUtils, LogicalPlanVisitor, and LogicalPlanWalker to see if there's any method implemented which counts operations - as if such a method already exists I wouldn't want to reimplement the wheel, but it doesn't seem like there is one.
If there isn't such a method to count operations, then should I implement it as a member for the Randomized algorithm or should I add it to the LogicalPlanUtils or another package? I'm assuming that a logical plan and its subplans is a tree without cycles, so that I can visit a subplan to see its subplans, and so on, and in that way count the number of operators (are all operators binary?).

I'm thinking of whether I should be implementing an equilibrium condition for the SA algorithm, similar to how the II algorithm has a stopping condition, and also how I should best pass the number of operators to the optimize method, as the optimize method itself only deals with the physical plan and not the logical plan.

Yeah, good idea!

I don't know what difference it would make, but we could also count operations in the physical plan instead of counting them in the logical plan.

It would not make much difference, assuming you would always be using only the initial physical plan. Other physical plans that are semantically equivalent to the initial one (i.e., "neighbors" and "neighbors' neighbors") may have different numbers of operators. Therefore, to have a constant, the initial physical plan would need to be used. However, getting this number based on the logical plan may be easier in our current implementation because of the visitor pattern that is already implemented for them, as you have found ...

Regardless, I've looked into LogicalPlanUtils, LogicalPlanVisitor, and LogicalPlanWalker to see if there's any method implemented which counts operations - as if such a method already exists I wouldn't want to reimplement the wheel, but it doesn't seem like there is one.

Good thinking. Thanks for taking initiative!
You are right, we do not yet have such a counting functionality.

If there isn't such a method to count operations, then should I implement it as a member for the Randomized algorithm or should I add it to the LogicalPlanUtils or another package?

LogicalPlanUtils would be the best place to put this. So, similar to the SourceAssignmentChecker that is already there, you add another class that implements LogicalPlanVisitor and that simply maintains a counter which it increases by one in all of its visit methods. Then, you add another method similar to isSourceAssignment and in that method you pick out the counter in the end from the visitor.

I'm assuming that a logical plan and its subplans is a tree without cycles, so that I can visit a subplan to see its subplans, and so on, and in that way count the number of operators (are all operators binary?).

Exactly.

I've ran into a bit of a conundrum.
The equilibrium condition is based on a scaling factor, as in, the equilibrium is a generation count of x times the number of subplans in a logical plan.
The issue is where to store the amount of subplans.
If the number of subplans is to be found when the equilibrium condition is constructed and stored in the condition, then you essentially need a different condition for each plan, and therefore the same optimizer object cannot be used well for different plans. Therefore we must exclude this from consideration.
If the number of subplans is to be found when optimize is ran, we must figure this out when optimize(logicalplan) is ran and then pass it on to a new optimize(physicalplan, numberOfSubplans) method. This would preclude SimulatedAnnealing from implementing the base optimize(physicalplan) method found in RandomizedQueryOptimizerBase.
We could decide this in optimize(PhysicalPlan) by finding a way to get the number of subplans from a physical plan rather than a logical plan, but I do not know how efficient this would be.
We could remove the optimize(PhysicalPlan) method and add its functionality into optimize(LogicalPlan) since optimize(LogicalPlan) just calls optimize(PhysicalPlan) anyway, but since the optimize(PhysicalPlan) method is public I assume you want some means of working directly on a PhysicalPlan from the outside.
What do you think? @hartig

Yes, SimulatedAnnealing should implement optimize(logicalplan) by calling a new optimize(physicalplan, numberOfSubplans) method. Hence, it turns out that implementing optimize(logicalplan) in the abstract base class is too generic and does not work.

There seems to be something done with the base class in main, I'll leave it to you to resolve the conflict since I'm not clued in to what you've been using the base class for elsewhere.
Since SimulatedAnnealing no longer implements optimize(physicalPlan) but instead has optimize(physicalPlan, numberOfSubplans), the base class no longer declares an abstract optimize(physicalPlan). It's commented out for now.
SA is ready now and II still succeeds with the test. I will create a new branch for 2PO as soon as you've approved the merge.
Overall it's been more work than initially expected as helper methods needed to be made and so on, but it's been really fun to learn the IDE and the language, and I'm looking forward to seeing what you have in store for me once 2PO is completed at the end of this year/start of the next. Merry Christmas, by the way!

I have merged this PR now.
Two things are remaining to complete the work on implementing the paper:

Change the existing code of IterativeImprovementBasedQueryOptimizer to use the randomized join plan optimizer.
Implement the third algorithm (2PO).

I suggest you do 1 next. To this end, notice that RandomizedJoinPlanOptimizerImpl is only for subplans that consist solely of binary joins. Therefore, you need to create a SimpleJoinOrderingQueryOptimizer with a RandomizedJoinPlanOptimizerImpl as its join plan optimizer.


Stereo separation

Some way to set stereo separation would be nice.

Yeah. This is coming in 0.2

This is now included in the 0.2 release. Notice this setting is currently global only, but it will be possible to tweak on a per file/directory/type/path/etc setting later on.


Pull https://resume.github.io/ data into user profile

We can use https://resume.github.io/ code at the backend and then use the data generated by this program to be pushed into the user profile. We can check the feasibility of doing this.
This will required change sin user-schema in mongoose.

Done! Successfully incorporated resume.github.io source code into the app.


fix: Handle encoding issues in API response body

Description
This PR addresses encoding issues that occur when processing API responses by implementing proper UTF-8 encoding handling.
Changes

Added error handling for Encoding::UndefinedConversionError
Force UTF-8 encoding on response body when encoding issues are detected

Why
Some API responses may contain special characters or different encodings that cause conversion errors. This fix ensures proper handling of such responses by forcing UTF-8 encoding when necessary.

Gracias por tu aporte


How to grab an interactable object in a precise position

When you grab an interactable, it snaps the centre of the object to the grab point on the controller. A tutorial showing how to precisely grab the position where the controller touched the object.

https://www.youtube.com/watch?v=dBWoiO_cLfM


Check if problem-json is content type for error status codes

If https://github.com/swagger-api/swagger-parser/pull/303/files is merged, we can check actual content type.

The issue was closed with:
Hi, we don't support this in the spec, so have to punt in the parser :( Consider adding an extension to do this, and I'd be happy to add it to the repo as well.

Most probably we need a plan B :-)

Did few requests against latest master:
curl -i -X GET http://localhost:8080/
HTTP/1.1 405
X-Application-Context: application:dev
Allow: POST
Content-Type: application/problem+json
Transfer-Encoding: chunked
Date: Thu, 09 Mar 2017 21:41:15 GMT

{"title":"Method Not Allowed","status":405,"detail":"Request method 'GET' not supported"}

curl -i -X POST http://localhost:8080/
HTTP/1.1 400
X-Application-Context: application:dev
Content-Type: application/problem+json
Transfer-Encoding: chunked
Date: Thu, 09 Mar 2017 21:42:12 GMT
Connection: close

{"title":"Bad Request","status":400,"detail":"Required request body is missing: public org.springframework.http.ResponseEntity<com.fasterxml.jackson.databind.JsonNode> de.zalando.zally.ApiViolationsController.validate(com.fasterxml.jackson.databind.JsonNode)"}

curl -i -X POST -H "Content-Type: application/json" http://localhost:8080/ -d '{"abcde": 123'
HTTP/1.1 400
X-Application-Context: application:dev
Content-Type: application/problem+json
Transfer-Encoding: chunked
Date: Thu, 09 Mar 2017 21:43:30 GMT
Connection: close

{"title":"Bad Request","status":400,"detail":"Could not read document: Unexpected end-of-input: expected close marker for Object (start marker at [Source: java.io.PushbackInputStream@4a44189f; line: 1, column: 1])\n at [Source: java.io.PushbackInputStream@4a44189f; line: 1, column: 40]; nested exception is com.fasterxml.jackson.core.io.JsonEOFException: Unexpected end-of-input: expected close marker for Object (start marker at [Source: java.io.PushbackInputStream@4a44189f; line: 1, column: 1])\n at [Source: java.io.PushbackInputStream@4a44189f; line: 1, column: 40]"

It's working as expected. Closing this issue.

Sorry, my misunderstanding. Reopening.

I've tried to find other PRs to swagger repos from this guy, no chance.
@fmueller should we postpone it and move out of the scope of MS 1.0?

@netme Good point, I don't think it makes sense to keep it in the scope of version 1.0. We'll postpone it.
Thanks for following up ;)

Implemented in #426


create ionic app

create stock ionic app with angular base, probably plain projekt

Creating a new ionic Projekt in our Repo


Removing LDAP config from uaa.yml [KETI-1688]

No need for the change in uaa.yaml here, we use the one from the provisioning repo which has the LDAP change.


train errors

when i tried to run train.py i am getting the below error. please help me to slove it.if any one knew the answer please let me know
Traceback (most recent call last):
File "/home/mcis-lap-40/raviwork/Tensorflow_Models/text-to-image-master/train.py", line 256, in
main()
File "/home/mcis-lap-40/raviwork/Tensorflow_Models/text-to-image-master/train.py", line 106, in main
loaded_data)
TypeError: 'NoneType' object is not iterable

Use (TensorFlow) version 0.11.0 -- u can resolve the errors.


Added type annotations to TypeScript copy of file

Trivial change but I needed it for my project so offering it to the trunk

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Heads up, there are some changes in the JS peer for PRs #4 and #5.
@mlhDevelopment, thanks for your contribution! You'll need to first complete the CLA before we could accept the patch.

@mlhDevelopment , I followed up with some of our developers who don't feel we can accept this patch as k6 does not support typescript at this time.
Thank you again, but I'll be closing this PR.

Hey, just came across this because I have need for this as well.
I appreciate that k6 itself doesn't support Typescript but they do provide an official template for using k6 with typescript and there is a types package for k6 itself.


Fix for proper toolbar hiding when leaving Unprotected Sites view

Task/Issue URL: https://app.asana.com/0/414709148257752/1201988294830333/f
Tech Design URL:
CC:
Description:
When navigating to the "Unprotected Sites" settings screen and back the bottom toolbar is still visible, but shouldn't.
Steps to test this PR:

Open Settings view
Open "Unprotected Sites"
Tap back button to go back to Settings

Before
After

Device Testing:

[ ] iPhone
[ ] iPad

OS Testing:

[ ] iOS 13
[ ] iOS 14
[ ] iOS 15

Internal references:
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

Tested the latest fix, smooth animations! LGTM 👍


Include genomes of potential contaminants during the host-removal stage.

Regarding --hostremoval_reference, currently, we only use GCF_000001405.39_GRCh38.p13_genomic.fna.
TO DO
1: Should we substitute GRCh38.p13 with the recently assembled human genome GCF_009914755.1_T2T-CHM13v2.0_genomic.fna? Alternatively, should we utilize both versions of the human genome?
2: Additionally, should we contemplate incorporating genomes of other potential contaminants? For example, a frequent contaminant in Illumina sequencing involves a spike-in control of the PhiX virus genome; Equine Infectious Anemia Virus (EIAV)
curl -O https://ftp.ncbi.nlm.nih.gov/genomes/all/GCF/000/819/615/GCF_000819615.1_ViralProj14015/GCF_000819615.1_ViralProj14015_genomic.fna.gz

Agreement: We will not include genomes of any potential contaminants during the host-removal stage, as it won't affect the classification results. However, it would be beneficial to investigate regular or common contaminations, similar to what Robert did for 16S.


Compilation instructions

Missing libraries make it hard for me to compile as there is no indication as to the proper versions or where to get i am able to get arm patch, have no idea about the rest. maybe add a download script?

You need to put xDL files into the folder xDL
You need libCURL compiled with WolfSSL because it is lightweght and opensource. And put them into needed folder.
If i miss something, just check this file: https://github.com/RusJJ/AndroidModLoader/blob/main/Android.mk

Yes but where is xDl from

That's what i am missing

That's what i am missing

https://github.com/hexhacking/xDL

Im getting a missing header
GTASA_structs.h
Is that a hidden header?

How is that related to AndroidModLoader?

Its not directly i looked through the whole organization for this header and find 12 references but no headers, since it isn't here and multiple mods reference it i figured it might be missing from the mod folder for aml

Now automatically compilable (even though cURL, wolfSSL and libz arent automatically compiled now because they are kinda ugh to build)


[CRT] Clear Compiler Warnings

During the investigation of https://github.com/apache/incubator-tvm/pull/5293, we find that the CRT source currently generate multiple warnings in both 64 bit and 32 bit env.
It is worth to look into and eliminate all the warnings, and turn on the -Werror compiler for future compilations

cc @liangfu @mehrdadhe  @tmoreau89


Refactor Redis commands and connection pool

This is part of the effort to build a proxy for Redis Cluster (https://github.com/envoyproxy/envoy/issues/5697).
The new proxy will live in a folder next to the old Redis proxy, as opposed to simply being a config change or flag. The rationale for creating two separate proxies is as follows. Redis Cluster has additional commands which will be internal to this proxy, which essentially functions as a smart client. It needs to maintain the cluster topology as well as sending commands to nodes to go from readonly to readwrite mode on change from replica to master. As we develop the new proxy we prefer to keep the changes separate from the old one so as not to affect other users. We can always merge the two later on.
In this PR, we move common Redis code (connection pools, command splitting, etc.) to a common folder, and change the related dependencies in redis_proxy, healthcheck and tests. In future PRs we will reuse the common code in the new proxy.
FYI @HenryYYang @mattklein123 @danielhochman

🔨

@FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg generally LGTM. Please merge master and I will take a look.
/wait

@FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg apologies but you will need to fix DCO. Sorry for the trouble. It might be worth getting a few tips from @HenryYYang if you are sitting next to him.
/wait

Yeah, I'm looking at it :|

Coverage has a (new) failure with a test unrelated to my changes. @mattklein123 I'll take a look at this after lunch.
test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc:505: Failure
Expected equality of these values:
  trace.socket_buffered_trace().events().size()
    Which is: 0
  2
[  FAILED  ] IpVersions/SslTapIntegrationTest.TruncationWithMultipleDataFrames/IPv4, where GetParam() = 4-byte object <00-00 00-00> (68 ms)

@FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg merge master again, I just fixed that flake.

The implementation of InstanceImpl in conn_pool_impl.h and conn_pool_impl.cc are specific to redis proxy and it incompatible with redis cluster, we should leave them in the redis proxy filter.

@HenryYYang per offline convo please review first and then I will take a final pass.
/wait

I believe we are going with the other opened PR so closing this one.


distortion in the unpatchified image

The original image is in shape (844,1280) and I patched it with patchify using 128 steps , because I need like +40 patches with size of (256,256) , unpatchify succeeded but sadly I have distortions in the result , is there a way to fix this ?
patched = patchify(image,(256,256),step=128)
unpatched_img = unpatchify(patched, image.shape)

the problem was solved by making the width and height the same for example 1024 X 1024 with 128 steps
because (1024 -256) mod 128 = 0


feat: add possibility for unordered ica channels #ntrn-376

Now ICA accounts can be with unordered channels.
They do not close after timeout. And messages be delivered in random order.
PR's:

https://github.com/neutron-org/neutron/pull/656
https://github.com/neutron-org/neutron-sdk/pull/160
https://github.com/neutron-org/neutron-dev-contracts/pull/62
https://github.com/neutron-org/neutron-integration-tests/pull/337

https://github.com/neutron-org/neutron-tests/actions/runs/10410952529

https://github.com/neutron-org/neutron-tests/actions/runs/10410952529


Add ncform

This is the front-end open source project of VIP

OK, I will provide a  english version README.
and ncform isn't a playground, it's a vue component, playground is a place to show ncform's features only

close due to in-activity.
if you want to add to list, you can re-send PR.


Default nginx config?

Hi there, was curious if you had a default nginx configuration file for a site to reference?
https://codex.wordpress.org/Nginx - has some good stuff, but curious what you use for your site's conf.

My configuration is based on what you can find in the Codex. I just added the following documentation:
https://github.com/maximejobin/rocket-nginx/wiki/Nginx-configuration-for-WordPress
Enjoy!

Thx @maximejobin !!

Thank you!


GIS data for beats / districts, geocoding

[Just an idea for future improvement]
ChicagoPoliceBeats (1).zip found here: http://edrub.in/ARE212/Spring2017/section12.html
a shapefile of beats. We could include this info to convert beats / districts into rough lat/long coordinates.
Another idea is to geocode the addresses in the complaint/TRR data to attach them to districts/beats.

So beat number is already given with the complaint and TRR data. But more generally, I agree that some form of normalization and/or geocoding of the addresses across datasets would be nice. Rather than the above source, I think this one is more authoritative: https://data.cityofchicago.org/Public-Safety/Boundaries-Police-Beats-current-/aerh-rz74
Also, the dataset of building footprints looks very useful: https://data.cityofchicago.org/Buildings/Building-Footprints-current-/hz9b-7nh8
I think in most cases, we can just lookup the address in the building data and get the gps coordinates directly from there, without using a geocoding service.


Publish egui 0.29 version

Hi there!
The version on crates.io uses egui 0.28, but main uses egui 0.29. Would you be able to release a new version of the crate to crates.io using 0.29? Thanks!

Published 0.7.0, thanks!


(404) task level hints and guidance

Changes
Add hints and guidance to Task model
Update the workflow and TaskListCreator to handle hints and guidance
The TaskListCreator and workflow need updating to handle task-level hints and guidance. Update them, following the sam structure as used for the action-level hints and guidance.
Because some of the keys of the workflow task are now optional, this also replaces any calls to fetch with [] in the TaskListCreator. fetch raises an error when the key isn't found, whereas [] does not.
Render task hints and guidance
We want to render the task-level hints and guidance as GOV.UK Frontend content.
Follow the same pattern as the action-level guidance, and call the GovukMarkdown gem with the workflow markdown.
Checklist

[x] Attach this pull request to the appropriate card in Trello.
[x] Update the CHANGELOG.md if needed.

 Tasks can have hint text and guidance


configure issues

There are 2 problems with the dependencies checks in the configure script:

Minor one: if we don't use a dependency, it is still checked and a warning is generated in the log. Ex: without specifying "--with-lua", the LUA dependency is still checked.
OK, it's a minor one but having "false positive" warnings is confusing and really leads to trouble because of problem 2

When a needed dependency is not found, the script simply ignores it and the module is compiled without the needed support. There is a (not highly visible) warning but you usually won't notice it before installing and running the module. Example: you only notice the problem at run-time when analysing your audit log and see the message "JSON support was not enabled"

I think that the correct way would be to check the dependencies only when they are needed and, if they're missing, stop the processing as it's an error.

Hi @marcstern,
Not sure how a user cannot see if the dependency was found or not. There is a pretty neat configuration summary. As detailed below.

Get you give us more details on it?

I'm speaking about v2.9x.
./configure --with-lua => command works (exit status 0), no warning.
The only thing we have is about 30 lines with "checking for ...". In the middle, we have
checking for lua install... no
configure: optional lua library not found
If with specify that we want to build the library with LUA (or anything else) support, it should break if it can't, not build a library without the support we requested.


Icons in Nearby list showing up when phone is set to English but not when set to Czech

I noticed that icons in the Nearby list do not work when I set my phone language to Czech. Steps to reproduce: set language to Czech, reopen app, load Nearby list.

FYI I dont remember these icons ever working in Czech. I was actually surprised to find them in English version :-)

Thanks for catching this! There is probably an issue with the mapping. AFAIK, we are currently mapping icons based on this query, do you see a reason why it might not work with non-English languages?
     # Get the class label in the preferred language of the user, or any other language if no label is available in that language.
     OPTIONAL {
       ?item p:P31/ps:P31 ?classId.
       OPTIONAL {?classId rdfs:label ?class_label_preferred_language. FILTER (lang(?class_label_preferred_language) = "${LANG}")}
       OPTIONAL {?classId rdfs:label ?class_label_any_language}

The problem is at https://github.com/commons-app/apps-android-commons/blob/f1f41e59f7b513d071797bd0fd0eedc1df1dcbf9/app/src/main/java/fr/free/nrw/commons/nearby/Place.java#L122
We should get the QID of the class, and match against this QID. For instance river is Q4022.

I will take this issue, looks it's a general problem, acts the same for me in Slovak localization

Sure @Rulino please take this up. :)

Fixed in #1947


Sending a message before the FlutterEngine has been run.

[x] I have read the Get Started - Installation section
[x] I have read and done the Get Started - Setup Android section
[x] I have read and done the Get Started - Setup iOS section
[x] I have already searched for the same problem

Environment

Technology
Version

Flutter version
2.8.1

Plugin version
5.3.2

Android version
N/A

iOS version
12.4.1

Xcode version
13.2.1

Device information:
iPhone 7
Model:D101AP
Description
Expected behavior:
No crash
Current behavior:
App is crashing with the attached stacktrace.
Steps to reproduce
Cause app to go to background. Issue occurred in less than a second.
Actions performed by user seem to be unrelated to screens containing inappwebview widgets.
Based on breacrumbs from an anonymous, automated report.
Images
Stacktrace/Logcat
OS Version: iOS 12.4.1 (16G102)
Report Version: 104

Exception Type: EXC_CRASH (SIGABRT)
Crashed Thread: 0

Application Specific Information:
Sending a message before the FlutterEngine has been run.

Thread 0 Crashed:
0   CoreFoundation                  0x307fcc98c         <redacted>
1   libobjc.A.dylib                 0x3064919f8         objc_exception_throw
2   CoreFoundation                  0x307ee6098         <redacted>
3   Foundation                      0x309414b68         <redacted>
4   Flutter                         0x103111404         <redacted>
5   flutter_inappwebview            0x102d4d01c         $s20flutter_inappwebview12InAppWebViewC22onUpdateVisitedHistory3urlySSSg_tF
6   flutter_inappwebview            0x102d5cb4c         $s20flutter_inappwebview12InAppWebViewC12observeValue10forKeyPath2of6change7contextySSSg_ypSgSDySo05NSKeyh6ChangeJ0aypGSgSvSgtFTf4nnndn_n
7   flutter_inappwebview            0x102d35c30         $s20flutter_inappwebview12InAppWebViewC12observeValue10forKeyPath2of6change7contextySSSg_ypSgSDySo05NSKeyh6ChangeJ0aypGSgSvSgtFTo
8   Foundation                      0x3094834e8         <redacted>
9   Foundation                      0x30948587c         <redacted>
10  Foundation                      0x309482d9c         <redacted>
11  WebKit                          0x326c3da74         <redacted>
12  WebKit                          0x326c3d89c         <redacted>
13  WebKit                          0x326c9d914         <redacted>
14  WebKit                          0x326c3efc4         <redacted>
15  WebKit                          0x326c3ed1c         <redacted>
16  WebKit                          0x326c925c0         <redacted>
17  flutter_inappwebview            0x102d5164c         $s20flutter_inappwebview12InAppWebViewC7disposeyyF
18  flutter_inappwebview            0x102d1e1c8         $s20flutter_inappwebview24FlutterWebViewControllerC7disposeyyF
19  flutter_inappwebview            0x102d1e344         $s20flutter_inappwebview24FlutterWebViewControllerCfDTo
20  Flutter                         0x103119b40         <redacted>
21  Flutter                         0x10311b4d8         <redacted>
22  Flutter                         0x103114224         <redacted>
23  Flutter                         0x10310f284         <redacted>
24  CoreFoundation                  0x307f3da28         <redacted>
25  CoreFoundation                  0x307f3d9f4         <redacted>
26  CoreFoundation                  0x307f3cee8         <redacted>
27  CoreFoundation                  0x307f3cb94         <redacted>
28  CoreFoundation                  0x307eb6474         <redacted>
29  CoreFoundation                  0x307f3c644         _CFXNotificationPost
30  Foundation                      0x3093901d4         <redacted>
31  UIKitCore                       0x360ce9f2c         <redacted>
32  UIKitCore                       0x3605a5d7c         <redacted>
33  UIKitCore                       0x360cee0ac         <redacted>
34  UIKitCore                       0x360cdcbfc         <redacted>
35  UIKitCore                       0x360d09a6c         <redacted>
36  CoreFoundation                  0x307f5dd08         <redacted>
37  CoreFoundation                  0x307f58a30         <redacted>
38  CoreFoundation                  0x307f58fac         <redacted>
39  CoreFoundation                  0x307f587c0         CFRunLoopRunSpecific
40  GraphicsServices                0x30c3f479c         GSEventRunModal
41  UIKitCore                       0x360ce2c38         UIApplicationMain
42  <Stripped name>                 0x2024acf1c         <redacted>
43  libdyld.dylib                   0x3075838e0         <redacted>

Thread 0 Crashed:
0   CoreFoundation                  0x307fcc98c         <redacted>
1   libobjc.A.dylib                 0x3064919f8         objc_exception_throw
2   CoreFoundation                  0x307ee6098         <redacted>
3   Foundation                      0x309414b68         <redacted>
4   Flutter                         0x103111404         <redacted>
5   flutter_inappwebview            0x102d4d01c         $s20flutter_inappwebview12InAppWebViewC22onUpdateVisitedHistory3urlySSSg_tF
6   flutter_inappwebview            0x102d5cb4c         $s20flutter_inappwebview12InAppWebViewC12observeValue10forKeyPath2of6change7contextySSSg_ypSgSDySo05NSKeyh6ChangeJ0aypGSgSvSgtFTf4nnndn_n
7   flutter_inappwebview            0x102d35c30         $s20flutter_inappwebview12InAppWebViewC12observeValue10forKeyPath2of6change7contextySSSg_ypSgSDySo05NSKeyh6ChangeJ0aypGSgSvSgtFTo
8   Foundation                      0x3094834e8         <redacted>
9   Foundation                      0x30948587c         <redacted>
10  Foundation                      0x309482d9c         <redacted>
11  WebKit                          0x326c3da74         <redacted>
12  WebKit                          0x326c3d89c         <redacted>
13  WebKit                          0x326c9d914         <redacted>
14  WebKit                          0x326c3efc4         <redacted>
15  WebKit                          0x326c3ed1c         <redacted>
16  WebKit                          0x326c925c0         <redacted>
17  flutter_inappwebview            0x102d5164c         $s20flutter_inappwebview12InAppWebViewC7disposeyyF
18  flutter_inappwebview            0x102d1e1c8         $s20flutter_inappwebview24FlutterWebViewControllerC7disposeyyF
19  flutter_inappwebview            0x102d1e344         $s20flutter_inappwebview24FlutterWebViewControllerCfDTo
20  Flutter                         0x103119b40         <redacted>
21  Flutter                         0x10311b4d8         <redacted>
22  Flutter                         0x103114224         <redacted>
23  Flutter                         0x10310f284         <redacted>
24  CoreFoundation                  0x307f3da28         <redacted>
25  CoreFoundation                  0x307f3d9f4         <redacted>
26  CoreFoundation                  0x307f3cee8         <redacted>
27  CoreFoundation                  0x307f3cb94         <redacted>
28  CoreFoundation                  0x307eb6474         <redacted>
29  CoreFoundation                  0x307f3c644         _CFXNotificationPost
30  Foundation                      0x3093901d4         <redacted>
31  UIKitCore                       0x360ce9f2c         <redacted>
32  UIKitCore                       0x3605a5d7c         <redacted>
33  UIKitCore                       0x360cee0ac         <redacted>
34  UIKitCore                       0x360cdcbfc         <redacted>
35  UIKitCore                       0x360d09a6c         <redacted>
36  CoreFoundation                  0x307f5dd08         <redacted>
37  CoreFoundation                  0x307f58a30         <redacted>
38  CoreFoundation                  0x307f58fac         <redacted>
39  CoreFoundation                  0x307f587c0         CFRunLoopRunSpecific
40  GraphicsServices                0x30c3f479c         GSEventRunModal
41  UIKitCore                       0x360ce2c38         UIApplicationMain
42  <Stripped name>                 0x2024acf1c         <redacted>
43  libdyld.dylib                   0x3075838e0         <redacted>

Thread 1 name: com.apple.uikit.eventfetch-thread
0   libsystem_kernel.dylib          0x3077f00f4         mach_msg_trap
1   libsystem_kernel.dylib          0x3077ef5a0         mach_msg
2   CoreFoundation                  0x307f5e120         <redacted>
3   CoreFoundation                  0x307f59030         <redacted>
4   CoreFoundation                  0x307f587c0         CFRunLoopRunSpecific
5   Foundation                      0x30939198c         <redacted>
6   Foundation                      0x30939181c         <redacted>
7   UIKitCore                       0x360dc8754         <redacted>
8   Foundation                      0x3094be4a0         <redacted>
9   libsystem_pthread.dylib         0x3079172c0         <redacted>
10  libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 2 name: io.worker.1
0   libsystem_kernel.dylib          0x3077faee4         __psynch_cvwait
1   libsystem_pthread.dylib         0x30790fcf8         <redacted>
2   libc++.1.dylib                  0x3063c2090         _ZNSt3__118condition_variable4waitERNS_11unique_lockINS_5mutexEEE
3   Flutter                         0x1033e8928         <redacted>
4   libsystem_pthread.dylib         0x3079172c0         <redacted>
5   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 3 name: io.worker.2
0   libsystem_kernel.dylib          0x3077faee4         __psynch_cvwait
1   libsystem_pthread.dylib         0x30790fcf8         <redacted>
2   libc++.1.dylib                  0x3063c2090         _ZNSt3__118condition_variable4waitERNS_11unique_lockINS_5mutexEEE
3   Flutter                         0x1033e8928         <redacted>
4   libsystem_pthread.dylib         0x3079172c0         <redacted>
5   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 4 name: dart:io EventHandler
0   libsystem_kernel.dylib          0x3077fc8f4         kevent
1   Flutter                         0x1035b14dc         <redacted>
2   Flutter                         0x1035e04e8         <redacted>
3   libsystem_pthread.dylib         0x3079172c0         <redacted>
4   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 5 name: JavaScriptCore bmalloc scavenger
0   libsystem_kernel.dylib          0x3077faee4         __psynch_cvwait
1   libsystem_pthread.dylib         0x30790fcf8         <redacted>
2   libc++.1.dylib                  0x3063c2128         _ZNSt3__118condition_variable15__do_timed_waitERNS_11unique_lockINS_5mutexEEENS_6chrono10time_pointINS5_12system_clockENS5_8durationIxNS_5ratioILl1ELl1000000000EEEEEEE
3   JavaScriptCore                  0x3164a1304         <redacted>
4   JavaScriptCore                  0x3164a11b4         <redacted>
5   JavaScriptCore                  0x31649fe4c         <redacted>
6   JavaScriptCore                  0x31649f54c         <redacted>
7   JavaScriptCore                  0x3164a0f8c         <redacted>
8   libsystem_pthread.dylib         0x3079172c0         <redacted>
9   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 6
0   libsystem_kernel.dylib          0x3077fb400         __semwait_signal
1   libsystem_c.dylib               0x30769256c         nanosleep
2   libsystem_c.dylib               0x30769236c         sleep
3   Sentry                          0x102b7aafc         monitorCachedData
4   libsystem_pthread.dylib         0x3079172c0         <redacted>
5   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 7 name: SentryCrash Exception Handler (Secondary)
0   libsystem_kernel.dylib          0x3077f00f4         mach_msg_trap
1   libsystem_kernel.dylib          0x3077ef5a0         mach_msg
2   libsystem_kernel.dylib          0x3077eb780         thread_suspend
3   Sentry                          0x102b87e1c         handleExceptions
4   libsystem_pthread.dylib         0x3079172c0         <redacted>
5   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 8 name: SentryCrash Exception Handler (Primary)
0   libsystem_kernel.dylib          0x3077f00f4         mach_msg_trap
1   libsystem_kernel.dylib          0x3077ef5a0         mach_msg
2   Sentry                          0x102b87e48         handleExceptions
3   libsystem_pthread.dylib         0x3079172c0         <redacted>
4   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 9 name: com.apple.NSURLConnectionLoader
0   libsystem_kernel.dylib          0x3077f00f4         mach_msg_trap
1   libsystem_kernel.dylib          0x3077ef5a0         mach_msg
2   CoreFoundation                  0x307f5e120         <redacted>
3   CoreFoundation                  0x307f59030         <redacted>
4   CoreFoundation                  0x307f587c0         CFRunLoopRunSpecific
5   CFNetwork                       0x308c2f74c         <redacted>
6   Foundation                      0x3094be4a0         <redacted>
7   libsystem_pthread.dylib         0x3079172c0         <redacted>
8   libsystem_pthread.dylib         0x307917220         _pthread_start

Thread 10
0   libsystem_kernel.dylib          0x3077fbb74         __workq_kernreturn
1   libsystem_pthread.dylib         0x307918138         _pthread_wqthread

Thread 11
0   libsystem_kernel.dylib          0x3077fbb74         __workq_kernreturn
1   libsystem_pthread.dylib         0x307918138         _pthread_wqthread

Thread 12
0   libsystem_kernel.dylib          0x3077fbb74         __workq_kernreturn
1   libsystem_pthread.dylib         0x3079181f8         _pthread_wqthread

Thread 13
0   libsystem_kernel.dylib          0x3077fbb74         __workq_kernreturn
1   libsystem_pthread.dylib         0x307918138         _pthread_wqthread

Thread 14
0   libsystem_kernel.dylib          0x3077fbb74         __workq_kernreturn
1   libsystem_pthread.dylib         0x307918138         _pthread_wqthread

Thread 15
0   libsystem_kernel.dylib          0x3077faee4         __psynch_cvwait
1   libsystem_pthread.dylib         0x30790fd2c         <redacted>
2   Flutter                         0x1036a221c         <redacted>
3   Flutter                         0x1036e8634         <redacted>
4   Flutter                         0x1036a1b44         <redacted>
5   libsystem_pthread.dylib         0x3079172c0         <redacted>
6   libsystem_pthread.dylib         0x307917220         _pthread_start

I have published the new version 5.4.1. Could you try it to check if the crash is gone now?

OK thx, I'll update flutter_inappwebview in my app ASAP. However, due to the fact, that I could not reproduce this issue I also can't check the fix directly. I can only monitor crash reports.

Sure! Thanks! 👍


CodeFeatures not being generated

Hi,
I'm trying to run Coming in feature mode. I installed the project with maven. When I run java -classpath ./target/coming-0-SNAPSHOT-jar-with-dependencies.jar fr.inria.coming.main.ComingMain -location ./files2compare -input files -mode features -output ./out. I get context and repair pattern features but code features are not being generated. Also, I'm getting errors/warnings below,
log4j:WARN File option not set for appender [file].
log4j:WARN Are you using FileAppender instead of ConsoleAppender?
log4j:ERROR No output stream or file set for the appender named [file].
java.lang.NullPointerException: Cannot invoke "Object.toString()" because the return value of "spoon.reflect.code.CtInvocation.getTarget()" is null
        at fr.inria.coming.codefeatures.codeanalyze.LogicalExpressionAnalyzer.analyzeLE7_VarDirectlyUsed(LogicalExpressionAnalyzer.java:684)
        at fr.inria.coming.codefeatures.codeanalyze.LogicalExpressionAnalyzer.analyze(LogicalExpressionAnalyzer.java:59)
        at fr.inria.coming.codefeatures.CodeFeatureDetector.analyzeFeatures(CodeFeatureDetector.java:50)
        at fr.inria.coming.codefeatures.CodeFeatureDetector.analyzeFeatures(CodeFeatureDetector.java:96)
        at fr.inria.coming.codefeatures.FeatureAnalyzer.analyze(FeatureAnalyzer.java:88)
        at fr.inria.coming.core.engine.RevisionNavigationExperiment.analyze(RevisionNavigationExperiment.java:127)
        at fr.inria.coming.main.ComingMain.start(ComingMain.java:147)
        at fr.inria.coming.main.ComingMain.run(ComingMain.java:140)
        at fr.inria.coming.main.ComingMain.main(ComingMain.java:124)
WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance.
14:01:00.084 [main] DEBUG add.features.detector.spoon.SpoonHelper - InsertOperation
14:01:00.085 [main] DEBUG add.features.detector.spoon.SpoonHelper - org.jfree.chart.renderer.AbstractRenderer

14:01:00.085 [main] DEBUG add.features.detector.spoon.SpoonHelper - Insert ...

Am I missing something?

it seems that one input file cannot be correctly parsed with Spoon.
could you try to add a guard at LogicalExpressionAnalyzer.java:684?

Thanks for the support. I applied the patch below
diff --git a/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java b/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
index 12051ac3..e1b47f87 100755
--- a/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
+++ b/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
@@ -681,7 +681,7 @@ public class LogicalExpressionAnalyzer extends AbstractCodeAnalyzer {

 				for (CtInvocation invocation : invocationssInStatement) {

-					if(!invocation.getTarget().toString().isEmpty()) {
+					if(invocation.getTarget() != null && !invocation.getTarget().toString().isEmpty()) {
 						continue;
 					}

I am not getting the NullPointerException anymore, but the code description features are still not being generated and there are still the following errors/warnings.
log4j:WARN File option not set for appender [file].
log4j:WARN Are you using FileAppender instead of ConsoleAppender?
log4j:ERROR No output stream or file set for the appender named [file].

Thanks for the support. I applied the patch below
diff --git a/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java b/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
index 12051ac3..e1b47f87 100755
--- a/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
+++ b/src/main/java/fr/inria/coming/codefeatures/codeanalyze/LogicalExpressionAnalyzer.java
@@ -681,7 +681,7 @@ public class LogicalExpressionAnalyzer extends AbstractCodeAnalyzer {

 				for (CtInvocation invocation : invocationssInStatement) {

-					if(!invocation.getTarget().toString().isEmpty()) {
+					if(invocation.getTarget() != null && !invocation.getTarget().toString().isEmpty()) {
 						continue;
 					}

I am not getting the NullPointerException anymore, but the code description features are still not being generated and there are still the following errors/warnings.
log4j:WARN File option not set for appender [file].
log4j:WARN Are you using FileAppender instead of ConsoleAppender?
log4j:ERROR No output stream or file set for the appender named [file].

this works for me.
	@Test
	public void testissue259() throws Exception {

		//java - classpath. / target / coming - 0 - SNAPSHOT - jar - with - dependencies.jar fr.inria.coming.main.ComingMain - location. / files2compare - input files - mode features - output. / out

		fr.inria.coming.main.ComingMain.main(new String[] { "-location", "./repogit4testv0", "-input", "git", "-output", "out", "-mode", "features" });

	}

can you try in input git instead of files?

I found that after adding the guard at LogicalExpressionAnalyzer.java:684,  Coming was actually generating the features correctly. I appreciate your support.

Great, could you send a pull-request with the fix?

I have created a pull request regarding this issue.


Port odometry publishing fixes to humble

Port of https://github.com/Ekumen-OS/Noah-bot-simulation-ros2/pull/11 to humble
And fix one issue remaining from https://github.com/Ekumen-OS/Noah-bot-simulation-ros2/pull/16
Part of #15

Superseded by https://github.com/Ekumen-OS/Noah-bot-simulation-ros2/pull/18


Add Github Action release pipeline

Is your feature request related to a problem? Please describe.
Deliver ready to use addon folder.
Describe the solution you'd like
Github action compile, run tests, and then publish a zip addon folder in a created release on tag push.
Addon folder contains libraries and is ready to use.
Describe alternatives you've considered
N/A

I'm experiencing trouble with release.
Copying addon folder into project only work on OSX...
Problem is described here.

Resolves by #27


(MAINT) Remove README_BRANCHING.md

Remove README_BRANCHING.md, as the documentation of our branching
strategy has moved over to the wiki.

:+1:


tmp: Add tmp.mount to kata-containers.target

Invoke tmp.mount by adding it to kata-containers.target.
This is not invoked by systemd with current rootfs setup we have
as one of the dependencies for tmp.mount is systemd-remount-fs.service
which depends on /etc/fstab file being present(it is currenty missing)
Instead of adding that file, start the tmp.mount unit by including
it in kata-containers.target
With this and change in os-builder to not delete the tmp.mount unit,
(https://github.com/kata-containers/osbuilder/pull/301) /tmp should now
be writeable for systemd based images.
For initrd this is handled by the agent itself.
Fixes #https://github.com/kata-containers/osbuilder/issues/300
Signed-off-by: Archana Shinde<EMAIL_ADDRESS>

I think the CI fixes check is not happy parsing your fixes line. I think we've seen this before eh @jodh-intel - @amshinde , you probably have to have an Issue in this repo to link to.
Fixes #kata-containers/osbuilder#300

Yep - please just specify Fixes: #XXX (or raise a PR on https://github.com/kata-containers/tests/tree/master/cmd/checkcommits to support the alternative syntax ;)

/test

18.04 CI is failing with:
[3] check dmesg logs errors
[3] /tmp/jenkins/workspace/kata-containers-agent-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:277
[3]   Run to check dmesg log errors
[3]   /tmp/jenkins/workspace/kata-containers-agent-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:294
[3]     should be empty [It]
[3]     /tmp/jenkins/workspace/kata-containers-agent-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:295
[3]
[3]     Expected
[3]         <string>: [    0.931156] systemd[71]: tmp.mount: Failed to execute command: No such file or directory
[3]         [    0.932756] systemd[71]: tmp.mount: Failed at step EXEC spawning /usr/bin/mount: No such file or directory
[3]     to be empty
[3]
[3]     /tmp/jenkins/workspace/kata-containers-agent-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:305

/test

@GabyCT I know the fedora tests were failing the other day. Are the vsocks tests supposed to pass ?

@amshinde the reason why vsocks is failing is because is using fedora which currently has issues

@jodh-intel @kata-containers/agent PTAL. I think this can be merged.

Agreed...


wip/6.0 Fix a concurrency testing case which could last forever

I found the issue while working on https://github.com/hibernate/hibernate-orm/pull/3587. The PR has been in testing phase for a couple of days without any sign to be terminated, :(

I closed this PR for I am not sure how to solve the issue. Seems Travis testing global timeout config is a much better solution.


SoundChannel#position reports seconds instead of milliseconds

import openfl.events.Event;
import openfl.display.Sprite;

class Main extends Sprite  {
	public function new() {
		super();
		var sound = openfl.Assets.getMusic("assets/mode.ogg");
		trace(sound.length);
		var channel = sound.play();

		addEventListener(Event.ENTER_FRAME, function(_) trace(channel.position));
	}
}

Sound used for testing: mode.ogg
OpenFL 4.2.0, Lime 3.2.1
On Neko, sound.length traces 8698 like expected. However, the sound position is in seconds instead of milliseconds:
Main.hx:11: 0
Main.hx:11: 0.02
Main.hx:11: 0.02
Main.hx:11: 0.02
Main.hx:11: 0.04
Main.hx:11: 0.06
Main.hx:11: 0.081
Main.hx:11: 0.101
Main.hx:11: 0.121
Main.hx:11: 0.121
[...]

The correct unit (ms) is reported on Flash, HTML5 and OpenFL legacy.

Now, this would be easy to work around in Flixel (just multiply by 1000 if !openfl_legacy) if it wasn't for the following issue:
If you set a startTime in sound.play() (for instance sound.play(500);), channel.position gets stuck at that:
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500
Main.hx:11: 500

So it sometimes reports seconds, sometimes milliseconds.

Thanks for the quick fix, appears to work as expected now! :)


Add Voting Power to Validators field in IBFTExtra

Add Voting Power to Validators field in IBFTExtra
Description
We are using Polygon Edge to bootstrap custom blockchain, but we want to add Voting Power = staked balance of a validator.
On the code level, we plan to modify the structure of the Validator struct adding VotingPower to it.  In this the IBFTExtra Validators' field would be modified as well, adding to it a BigInt representing voting power.
Another option we consider is just modifying the ContractValidatorStore. We would keep in it the Voting Power of all validators and will use it when calculating quorum and consensus.
I believe the second option is better because we would not increase the size of the extra data, but maybe there are some security issues with it.
My question is which option is better in your opinion?
Thanks in advance!
Your environment

Validators type
BLS Validators with contract store
The branch that causes this issue.
Commit SHA: ae795165d46f06cb68ded8c918ecef93d91be53c

Hi @R-Santev ,
The team has been notified about this request. I will get back to you as soon as I have updates.


Improve predictability of writeUtf8 performance

Motivation:
writeUtf8 can suffer from inlining issues and/or megamorphic call-sites on the hot path due to ByteBuf hierarchy
Modifications:
Duplicate and specialize the code paths to reduce the need of polymorphic calls
Result:
Performance are more stable in user code

Need to explain the context of this PR with https://github.com/eclipse-vertx/vert.x/pull/3462#issuecomment-647038310: when user code mix different ByteBuf subtypes or just has it's own stack depth, the -XX:MaxInlineLevel config (9 on jdk 8 and 15 on jdk 14) could prevent certain calls to be inlined causing unwanted bound checks, prevent vectorizations...hitting the stability of performance we see in the micro-benchmarks or just in an ideal world.
This duplication is aiming to makes the performance a bit more stable (sadly PlatformDependent::putByte has still some depth and CharSequence:.charAt too, but the benchmarks already shows some improvement.
I don't think I will left the benchmark changes to be there in the final PR because are too tied to how the JDK is configured.

I've tweaked the benchs and impl to shows what's the effects of these changes...
this PR:
Benchmark                             (direct)   Mode  Cnt        Score        Error  Units
ByteBufUtilBenchmark.nestedWriteUtf8      true  thrpt   20  1135949.136 ±  15508.180  ops/s
ByteBufUtilBenchmark.nestedWriteUtf8     false  thrpt   20  1115468.247 ±  65425.276  ops/s
ByteBufUtilBenchmark.writeUtf8String      true  thrpt   20  8237112.811 ± 274131.929  ops/s
ByteBufUtilBenchmark.writeUtf8String     false  thrpt   20  4164310.459 ± 117297.047  ops/s

Before this PR with -XX:MaxInlineLevel=9:
Benchmark                             (direct)   Mode  Cnt        Score        Error  Units
ByteBufUtilBenchmark.nestedWriteUtf8      true  thrpt   20   738208.471 ±  13030.643  ops/s
ByteBufUtilBenchmark.nestedWriteUtf8     false  thrpt   20   815001.393 ±  48192.092  ops/s
ByteBufUtilBenchmark.writeUtf8String      true  thrpt   20  5859888.733 ± 143176.688  ops/s
ByteBufUtilBenchmark.writeUtf8String     false  thrpt   20  4484238.436 ±  64584.530  ops/s

Before this PR with -XX:MaxInlineLevel=15:
Benchmark                             (direct)   Mode  Cnt        Score       Error  Units
ByteBufUtilBenchmark.nestedWriteUtf8      true  thrpt   20  5499785.717 ± 59616.413  ops/s
ByteBufUtilBenchmark.nestedWriteUtf8     false  thrpt   20  4954847.853 ± 70535.926  ops/s
ByteBufUtilBenchmark.writeUtf8String      true  thrpt   20  5826841.982 ± 66342.052  ops/s
ByteBufUtilBenchmark.writeUtf8String     false  thrpt   20  4597271.515 ± 17988.082  ops/s

on jdk8 nestedWriteUtf8 shows what's the possible effects of failing to inline some of the Netty buffer calls (or PlatformDependent too), while String::charAt isn't inlined (sadly, but could be fixed too on jdk 8 with some "hack").
writeUtf8String shows instead the effect of this PR while everything is correctly inlined instead, and performance are slightly improved too ;)
In this benchmark I haven't shows what's the effect of polymorphism (bi,tri, mega...) but I'm sure is needed, because just inlining shows the motivation behind the duplicated code.

I've pushed a further commit that is manually inlining the slow path of writeUtf8 ie writeUtf8Surrogate in order to make the methods > -XX:FreqInlineSize (ie 325 bytes)
eg io.netty.buffer.ByteBufUtil::unsafeWriteUtf8 (358 bytes)   hot method too big and
io.netty.buffer.ByteBufUtil::arrayWriteUtf8 (328 bytes)   hot method too big
The results seems nice:
Benchmark                             (direct)   Mode  Cnt        Score        Error  Units
ByteBufUtilBenchmark.nestedWriteUtf8      true  thrpt   20  6524381.794 ± 102816.039  ops/s
ByteBufUtilBenchmark.nestedWriteUtf8     false  thrpt   20  4101033.546 ± 238837.837  ops/s
ByteBufUtilBenchmark.writeUtf8String      true  thrpt   20  7958679.895 ± 687717.569  ops/s
ByteBufUtilBenchmark.writeUtf8String     false  thrpt   20  4502402.790 ±  91698.297  ops/s

But I would like to see how the ASM looks like

I'm not very used to achieve good numbers by playing so much with the JDK implementation details: I would like some good eyes to help me a bit on this (@nitsanw if he has the chance to look at this)

In the last commit I've tried to use PlatformDependent on byte[] to save the array bound-checks to happen and indeed I see some improvement: it couldn't be used before becaure now the xxxWriteUtf8 methods are not inlineable anymore...
Benchmark                             (direct)   Mode  Cnt        Score        Error  Units
ByteBufUtilBenchmark.nestedWriteUtf8      true  thrpt   20  7848114.042 ± 229818.691  ops/s
ByteBufUtilBenchmark.nestedWriteUtf8     false  thrpt   20  5907367.968 ± 321506.155  ops/s
ByteBufUtilBenchmark.writeUtf8String      true  thrpt   20  8264472.002 ±  49110.805  ops/s
ByteBufUtilBenchmark.writeUtf8String     false  thrpt   20  6166188.985 ± 336638.483  ops/s

The thing that isn't 100% convincing me is the magnitude of error that doesn't seem proportional to the gain: I still need to investigate what's going on.

I suppose @trustin could be interested into this as well :)

I know the code is based off of some of the hacks I did on this blog post : http://psy-lob-saw.blogspot.com/2012/12/encode-utf-8-string-to-bytebuffer-faster.html
But it's been a while :-), reading through the patch though, I recall having an implementation somewhere that used the same code for array and memory case, there should be no need for the code duplication. Also, the blog post had a better data set to use for the benchmark IIRC.

I've commented in past how sensitive ByteBuf access is in general to stack-depth due to transitive optimizations from inlining (eliminating accessibility checks). It means that in cases like this where there are many granular accesses, how deep the call stack happens to be makes an enormous difference to performance.
Having started to go down the rabbit hole of manually inlining parts of the ByteBuf hierarchy I came to the conclusion that it's not really worth trying to trim the stack depth by one or two calls for specific applications if that results in a lot of duplication and less-maintainable code, since it only helps in the borderline cases (the app in question might add an additional level of method indirection, at which point the benefit is lost).
Instead I feel we should just strongly advertise the fact that users can often get a big performance boost by setting -XX:MaxInlineLevel much higher than the default (at least for java < 14). I haven't encountered any cases where doing so is detrimental.
But otherwise I do really like the idea of speeding up reading/writing utf8 to ByteBufs if/where possible given its prevalence on critical paths.
One specific question about the changes is why inlining writeUtf8Surrogate makes sense given how rarely that path would be hit?

@njhill

One specific question about the changes is why inlining writeUtf8Surrogate makes sense given how rarely that path would be hit?

https://github.com/netty/netty/pull/10368#issuecomment-648217848
Tldr to save the big method from being inlined and "refill" the inlining budget: this would save from being dependent by the user code stack depth and would make any PlatformDependent call inlineable (because < 9th stack depth).
By consequence, CharSequence::chatAt became a bimorphic call for most usages (String or StringBuilder), getting inlined as well.

Thanks @franz1981, I guess maybe there's something I misunderstood about the inlining budget. I thought MaxInlineLevel was based on absolute stack depth (at bytecode level), so whether a parent method is inlined or not wouldn't affect the effective stack depth of its callees for this purpose. If you are saying that the depth counter gets reset to zero from any non-inlined method in the stack I would be very interested and happy to be mistaken! There's probably a bunch of other places that could benefit from similar parent method "inline blocking".
Though I don't follow why this affects whether charAt is considered bimorphic or not, and also don't follow why it's necessary to split into the various similar implementations since the main thing that matters afaik is that the max inlining depth is sufficient to collapse all of the descendent ByteBuf accessibility checks into the same level.

@njhill

If you are saying that the depth counter gets reset to zero from any non-inlined method in the stack I would be very interested and happy to be mistaken!

:) TBH I have tried it by writing some tests (similar to the one shown on this PR, with the nested calls), but that's why I needed the @nitsanw validation on this ;)
I'll try tomorrow to answer you back to the other questions you've rightly asked: I see that I have pushed into this PR much motivations that I haven't explained clearly :P

Both @franz1981 and @njhill are pointing to concrete factors in inlining considerations. Roughly speaking we have:

inlining depth (can configure, but people rarely do, I support Nick's recommendation and have seen in applied widely to good result), too deep and it won't get inlined. This depth is computed from the perspective of the method being compiled, not the base of the stack.
method size (also configurable), too large and it won't get inlined
failure to de-virtualize, e.g. due to a mega-morphic callsite

IIUC Franz is trying to improve inlining by manually unrolling call-sites and by creating a method large enough to prevent inlining. If you prevent inlining of method X you are effectively resetting the inlining budget/scope and allowing the compiler a better chance of inlining it's callees into it.
Shaping compilation at this level without inlining control (e.g. via CompileCommand) is brittle, but can be effective.
I hope this helps in clarifying the challenge/scope/tools :-)

Nitsan has perfectly summarised and understood the intention and motivation of this PR, thanks :)
Nonetheless he has confirmed the "inlining budged refill hack": the reason why I have performed tests comparing performance vs a tweaked MaxInliningLevel on 4.1 is that this PR should be at least on par (or better) then the original version regardless the hack to represent a step forward.
Most of this come from the code duplication that's peeling off the callsites to make each writeUt8 a monomorphic version (related to ByteBuf) of the original code, decoupled from the ByteBuf hierarchy.

https://github.com/nitsanw/jmh-samples/blob/master/src/main/java/psy/lob/saw/utf8/Utf8EncodingBenchmark.java
The benchmark I mentioned, which can still be improved

I've adapted (ie copied) the nice benchmark data of @nitsanw into a new benchmark and these are the results:
Post PR:
Benchmark                                                (direct)   Mode  Cnt      Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String             true  thrpt   20  13865.136 ± 262.175  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String            false  thrpt   20  12147.668 ± 426.339  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8StringBuilder      true  thrpt   20  12688.814 ±  38.461  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8StringBuilder     false  thrpt   20  11801.796 ± 147.851  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String       true  thrpt   20  12943.926 ± 133.867  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String      false  thrpt   20  10628.178 ±  98.822  ops/s

Pre PR:
Benchmark                                                (direct)   Mode  Cnt      Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String             true  thrpt   20  12712.676 ±  45.297  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String            false  thrpt   20  13256.125 ± 148.626  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8StringBuilder      true  thrpt   20  12622.236 ±  29.238  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8StringBuilder     false  thrpt   20  13119.969 ± 252.603  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String       true  thrpt   20   3000.798 ±  24.573  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String      false  thrpt   20   2500.261 ± 343.188  ops/s

I see that array based ByteBuf are now getting a small perf hit, while direct ones are improved: I need to go down into the rabbit hole to understand what's going on.
nestedByteBufUtilWriteUtf8String probably won't survice the PR when/if merged because too tied to the JDK version, but it shows the reason behind this PR: we're nearly getting a 5X improvement (!).

@njhill I think we have now some nice tool to play with :)
@njhill @normanmaurer @nitsanw
As a general rule of thumb, well optimized code always tends to play dirty with the JVM rules and we often try to pack cold/exceptional cases/branches away from the expected hottest path to make things inlineable. This PR is doing the same but for the opposite reason hence IMO it's fragile as much as the code it's going to replace.
The other optimization to reduce the influence of the ByteBuf hierarchy depth is performed in the form of the code duplication, that Nitsan correctly said could be saved for byte[] and off-heap case, given that from the Unsafe perspective they are the same, but it would require me to modify PlatformDependent. This last optimization is what makes the code uglier, but is probably what's making the direct ByteBuf case faster too.

Re

I see that array based ByteBuf are now getting a small perf hit

I see on the ASM that I'm getting some unexpected bound checks although using PlatformDependent, while if I use directly Unsafe and store in a local field the cached byte[] array offset I get back the original performance (and no bound checks):
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String           false  thrpt   20  13163.882 ± 160.442  ops/s

The latest commit shows the best performance (if compared to 4.1 too):
Benchmark                                               (direct)   Mode  Cnt      Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String            true  thrpt   20  13632.727 ± 350.050  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String           false  thrpt   20  13744.658 ±  86.133  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String      true  thrpt   20  11753.634 ± 326.259  ops/s
Utf8EncodingBenchmark.nestedByteBufUtilWriteUtf8String     false  thrpt   20  12387.934 ± 171.924  ops/s

@chrisvest may be interesting to you as well

Thanks @franz1981 @nitsanw for the very helpful additional clarifications and sorry for delayed reply, makes more sense to me now and I learnt something new. The megamorphic call-site concern is only w.r.t. ByteBuf rather than CharSequence, right?
I think the changes look great for the most part. Couple of other thoughts...

Instead of subtlety inflating the size of each of those methods to block inlining, how about just doing so explicitly in the parent method (i.e. top-level writeUtf8)? Then the writeUtf8Surrogate method could be de-duplicated again (by adding a ByteBuf to arg to the unsafe variants)
I wonder if the artifical inline-blocking could actually harm some use-cases, for example where the stack isn't very deep to start with and there are a large number of repeated calls to writeUtf8 with small char sequences?

Just thinking aloud but maybe it's possible to make this trick a bit more adaptive, where the stack depth is detected the first time that it's called and stored in a (plain) static flag. This and/or programmatic inspection of -XX:MaxInlineLevel could then be used to decide whether or not to pass through an artificially inflated inline-blocking method. I guess that might not work in cases like this where the stack depth could be very different between different calls in the same app.

@njhill

The megamorphic call-site concern is only w.r.t. ByteBuf rather than CharSequence, right?

Yes and it's one of the motivations for this PR (the primary was the inlineability of ByteBuf & PlatformDependent calls; that's related). Often people think that PlatformDependent calls are nearly free, like using Unsafe but the reality is that Unsafe methods are intrinstics while PlatformDependent ones still play a role in the inlining dance: this PR is aiming to save having a whole method call while accessing a single byte.
A negative note on this P, instead,  is related to potential magamorphism of CharSequence::charAt that could became a concern now, given that each specialized version of writeUtf8 isn't inlineable, charAt could see several implementors of CharSequence because the compiled method is shared across different callers AFAIK; but if there is a dominant type and the users will use it with < 3 types (String, StringBuilder?) I see JIT is still able to produce decent code ie hoisting the type check out of the loop and duplicating loop bodies to cover the bimorphic cases.
With >=3 I still need to check (and with JDK 11 is a real guess, given that String has got a wider hierarchy IIRC).

Instead of subtlety inflating the size of each of those methods to block inlining, how about just doing so explicitly in the parent method (i.e. top-level writeUtf8)? Then the writeUtf8Surrogate method could be de-duplicated again (by adding a ByteBuf to arg to the unsafe variants)

That's an option: I've tried to have 3 different method types to help with CharSequence polymorphism: each not-inlineable variant has it's own seen types on charAt level

I wonder if the artifical inline-blocking could actually harm some use-cases, for example where the stack isn't very deep to start with and there are a large number of repeated calls to writeUtf8 with small char sequences?

Would be nice to have a real case that heavily relies on writeUtf8: micro benchs sees that writing small char sequences can have a perf hit, but in the real world these ones still need to access some memory that could be cold, getting a cache miss on the first access: it should be dominant if compared with a full-fat method call, but only a real world case could help to understand it. Feel free to look at my benchs (borrowed from the nice ones of @nitsanw).

Just thinking aloud but maybe it's possible to make this trick a bit more adaptive, where the stack depth is detected the first time that it's called and stored in a (plain) static flag. This and/or programmatic inspection of -XX:MaxInlineLevel could then be used to decide whether or not to pass through an artificially inflated inline-blocking method. I guess that might not work in cases like this where the stack depth could be very different between different calls in the same app.

Ah it would be rather easy if we could control the inlineability of PlatformDependent too :)
But yes: i just wanted to not reduce dramatically the quality of the original code, using the same "trick" we use to get inlineable calls; I'm sure we could further improve it by detecting somehow the context by which the user is using this, but only if it worths (see previous points: I don't know yet in a real-case scenario with several small written strings how this PR would negatively impact the perf)

@njhill
In order to show my concern about the >2 case, I've modified the benchmark to shows the effect of bi/polymorphic cases of CharSequence args, results below:
this PR:
Benchmark                                              (direct)  (noUnsafe)   Mode  Cnt      Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Bimorphic       false       false  thrpt   10   4738.091 ±  40.236  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Bimorphic       false        true  thrpt   10   3784.695 ± 114.724  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Bimorphic        true       false  thrpt   10   4758.198 ±  66.403  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Bimorphic        true        true  thrpt   10   2242.832 ±  61.926  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Megamorphic     false       false  thrpt   10   1271.008 ±  22.136  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Megamorphic     false        true  thrpt   10   1168.745 ±   6.868  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Megamorphic      true       false  thrpt   10   1250.535 ±  59.894  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8Megamorphic      true        true  thrpt   10    991.959 ±   4.620  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String          false       false  thrpt   10  10486.611 ± 192.794  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String          false        true  thrpt   10   7484.192 ± 160.313  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String           true       false  thrpt   10  10719.207 ± 104.911  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8String           true        true  thrpt   10   4532.280 ± 280.026  ops/s

still awaiting 4.1 results

Hi @franz1981,
If I understand correctly, this prevents writeUtf8 from being inlined into callers, where buffer and seq could potentially monomorphise, and so instead manually specialises to recreate the opportunity to monomorphise those two, and allow their methods to inline?
The performance benefit looks kind of small, outside of the nested… noUnsafe=false cases, considering we now get 3 copies of the same code. It'd be more palatable if there were only two copies, along the safe/unsafe seam. Can you check and summaries the performance we'd get if we went with that?

To summarize, the motivations behind this pr was indeed to:

makes writeUtf8 more resilient to be used on user code due to inlining budged exhaustion while reaching the PlatformDependent and CharSequence methods -> enlarged writeUtf8 enough to refill the inlining budget
makes writeUtf8 more resilient to ByteBuf hierarchy, saving megamorphic calls -> specialized writeUtf8 in the 3 variants to cover all cases, decoupled by ByteBuf hierarchy

Results applying the solutions:

work well for nested calls but can cause megamorphic calls of CharSequence if the observed types > 2
works well and can be optimized with just 2 variant of the code too, merging the !PlatformDependent::hasUnsafe byte[] case into the AbstractByteBuf one (OpenJ9 users won't like it IMO)

My concern is if I should try to pursue 1) or not, given that most users just use String/StringBuilder...

There's also AsciiString that makes its way into this method (in StompSubframeEncoder.encodeFrame), but that was the only case I could find in Netty itself (and it easily (I would think?) disappears if we call toString on those headers, or specialise for AsciiString).

@chrisvest I've pushed some changes:

handling AsciiString differently: similarly to writeUtf8, decoupling it from the ByteBuf hierarchy, but NOT enlarging it enough to not be inlined (the code just won't fit that purpose "naturally")
adding some new benchmark method to show the difference with the version pre-PR

Soon the results

@franz1981 one solution to mega-morphic callsites is to strip out the known main case, e.g.:
void doSomething(CharSequence cs) {
  // expected 60% of the time, inlining to known type locations should eliminate the `instanceof`
  if (cs instanceof String) {
   // we're monomorphic here
  }
  else {
    // we now extended the callsite type budget by one, so this may end up bi-morphic, or mega-morphic, but at least the main case is covered
  }
}

this PR:
Benchmark                                                       (direct)  (noUnsafe)   Mode  Cnt      Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString             false       false  thrpt   10  24442.071 ± 704.774  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString             false        true  thrpt   10  25229.056 ± 339.069  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString              true       false  thrpt   10  24563.689 ± 451.031  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString              true        true  thrpt   10  14326.507 ± 105.995  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences     false       false  thrpt   10   3824.088 ± 28.556  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences     false        true  thrpt   10   2558.464 ± 26.097  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true       false  thrpt   10   3882.786 ± 71.585  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true        true  thrpt   10   1885.152 ± 19.069  ops/s

4.1:
Benchmark                                                      (direct)  (noUnsafe)   Mode  Cnt     Score     Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString             false       false  thrpt   10  13892.751 ±  97.401  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString             false        true  thrpt   10  13389.432 ±  65.051  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString              true       false  thrpt   10  15946.286 ± 244.559  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8AsciiString              true        true  thrpt   10   9161.213 ±  29.902  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences     false       false  thrpt   10   3539.553 ±  26.918  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences     false        true  thrpt   10   2833.111 ±   5.373  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true       false  thrpt   10   3667.355 ± 138.298  ops/s
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true        true  thrpt   10   1627.749 ± 942.380  ops/s

In summary: having a specialized AsciiString code path both reduce the chance to have > 2 CharSequence implementors and improve ASCII handling (for both write UTF-8 and write ASCII cases).

@nitsanw
re

// we're monomorphic here

I've just done it for AsciiString, but I'm not sure to do it for String/StringBuilder cases too or we would end having lot of copied methods...I mean, if it is for me, I will do it if the benefits are stable and better performance, but I understand if some of the Netty other committers won't like it too...

@netty-bot test this please

@netty-bot test this please

@franz1981 There are some checkstyle complaints. Please take a look!

@njhill @chrisvest @normanmaurer I've squashed the commits into one, addressed the checkstyle comments (and the Nick's one on written), but haven't yet fully used the good advice from @nitsanw .
Right now writeUtf8 can see a maximum of 2 CharSequence implementors (String, StringBuilder), because AsciiString is being handled in a separate ad-hoc method: if we care about a specific case (eg String with Unsafe) I can add another separate (big) method to handle it, further increasing the chance to not getting monomorphic call-sites if we still care about the user code stability (reminder: I've enlarged the byte code size of each specialized method to replenish the inlining depth budget)

@franz1981 looks like the average user running jdk8 and Netty would benefit from boosting -XX:MaxInlineLevel to something like 15?

@johnou Absolutely yes, assuming that user code is performing many ByteBuf content manipulations (as @njhill has pointed clearly on past issues/PRs)

@netty-bot test this please

@franz1981 Looking at https://github.com/netty/netty/pull/10368#issuecomment-661811217 I see a regression for the CommonCharSequences on heap, no unsafe case (A is after, B is before):
                    | (direct) |(noUnsafe) | A    | B    |
AsciiString         |    false |     false | 24.4 | 13.8 | kops/s
AsciiString         |    false |      true | 25.2 | 13.3 | kops/s
AsciiString         |     true |     false | 24.5 | 15.9 | kops/s
AsciiString         |     true |      true | 14.3 |  9.1 | kops/s
CommonCharSequences |    false |     false |  3.8 |  3.5 | kops/s
CommonCharSequences |    false |      true |  2.5 |  2.8 | kops/s <- regression
CommonCharSequences |     true |     false |  3.8 |  3.6 | kops/s
CommonCharSequences |     true |      true |  1.8 |  1.6 | kops/s

Worth looking into?

@nitsanw

CommonCharSequences |    false |      true |  2.5 |  2.8 | kops/s

Good eyes! The no Unsafe case is getting a regression because of this request from @chrisvest on https://github.com/netty/netty/pull/10368#issuecomment-661019850

It'd be more palatable if there were only two copies, along the safe/unsafe seam

That I've "interpreted" by not optimizing the code path byte[] & no unsafe: I can restore that method I've removed to restore the perf benefit

looks like the average user running jdk8 and Netty would benefit from boosting -XX:MaxInlineLevel to something like 15?

@johnou I would even suggest higher e.g. 20 or more

@nitsanw
Restoring the array + no unsafe variant I'm getting:
Benchmark                                                      (direct)  (noUnsafe)   Mode  Cnt     Score    Error  Units
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences     false        true  thrpt   10  3330.168 ± 88.673  ops/s

that's a nice speedup over master IMO

looks like the average user running jdk8 and Netty would benefit from boosting -XX:MaxInlineLevel to something like 15?

@johnou I would even suggest higher e.g. 20 or more

Do you have data supporting that? MaxInlineDepth=15 has gone through some testing by OpenJDK team and is the new default for 14. With more inlining we can potentially get more code duplication -> larger code blobs -> more icache misses/code cache exhaustion, it's not a simple "more is better" knob to tune ;-)

@nitsanw I would like to improve the direct and no unsafe method as well: I'll update my results here

@nitsanw
I've further improved the no unsafe and direct case too by using internalNioBuffer:
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true        true  thrpt   10  3510.155 ± 169.558  ops/s

while before it:
Utf8EncodingBenchmark.byteBufUtilWriteUtf8CommonCharSequences      true        true  thrpt   10   1885.152 ± 19.069  ops/s

I'm going to push this change too but probably need some help from @normanmaurer too while using internalNioBuffer because I'm not sure if is the right tool for the job

Do you have data supporting that?

@nitsanw no, it's just based on anecdotal observations, so thanks for the clarification/warning! I understand it is a trade-off but get the sense that the variance of optimal values across different apps is quite high, i.e. 15 may be optimal on average but there are certain cases where the benefit from larger values is huge. It would be neat if the JDK could be more adaptive somehow w.r.t. how deep it goes for a particular subtree... for example based on some some on-the-fly analysis of the benefit in terms secondary optimization potential (reorderings etc).

@njhill the bounded size of the overall blob, callee methods size, and virtual callsites all play a role in limiting inlining, the hard limit is there as a "sane" boundary. The JVM, as of now, does not have a feedback loop from potential adverse effects (icache/iTLB misses etc) to revising inlining policy. I'm not sure any compiler out there does TBH. It's an interesting topic for a paper, worth checking if any were written. IIRC some work has been done on hiphop to reduce TLB misses through repacking of their code cache, which my be a relevant comparison.
Having said all that, if you have concrete examples where upping the limit is a win I would LOVE to hear more :-)

@nitsanw I imagine it also boils down to the application (size and usage) not to mention hardware.. probably one of those knobs which needs to be tuned for the application / environment.

Here's an interesting read: https://arxiv.org/pdf/1810.00905.pdf :-)

@nitsanw isn't this PR itself a good example? If it wasn't for the artifical method inflation trick to reset the inlining budget then calling ByteBufUtil#writeUtf8 from a deep app call stack would be very costly, to a degree I'd presume outweighs any adverse affects you mention. It's a step change in perf once it reaches the "leaf" methods containing the checks to reorder/hoist/coalese. You could see something similar in netty I think with a very long channel pipeline and a codec which performs many granular ByteBuf reads/writes.
I don't understand why bounding size of overall blob isn't sufficient, or at least should make it "safe" for the default depth limit to be larger, i.e. to avoid precluding long chains of trivially sized methods. Or maybe the size limit could depend on the current depth, getting smaller as the depth increases (but ultimately able to go deeper).

Also just came across this which looks very relevant http://aleksandar-prokopec.com/resources/docs/prio-inliner-final.pdf in particular "adaptive decision thresholds" and "deep inlining trials".

Thanks for all your helps :)
I've improved the test coverage for the ascii case (that has been improved as well) and now this one is in a completed state: please @normanmaurer, have fun :D

@franz1981 You got some checkstyle violations in ByteBufUtilTest. (Build says there's one, but I think there's two)

@chrisvest  Just tried a local run and it seems just 1, now fixed: which one is the second one 4 u?

@franz1981 Hmm… I thought these two consecutive empty lines would also cause trouble, but apparently not? https://github.com/netty/netty/pull/10368/files#diff-21be031058403699535d72fa9ea7548fR45-R46

@chrisvest Indeed is strange mmm let me remove that too and we're fine :)

@chrisvest @normanmaurer @vietj Sadly I don't have some big machine around to try this, but would be nice to test this vs 4.1 on some Techempower text bench

Checkstyle is at it again:
16:05:45 [INFO] Starting audit...
16:05:45 [ERROR] /code/microbench/src/main/java/io/netty/microbench/buffer/Utf8EncodingBenchmark.java:44:9: Redundant 'public' modifier. [RedundantModifier]
16:05:45 Audit done.

@chrisvest I suppose I should better configure my IDE..strange...

This one should be ready (despite the amount of duplicated code) @normanmaurer @chrisvest feel free to re-review this; the perf numbers are now pretty good and better/on-par with the original code

@netty-bot test this please

Hey @franz1981, can you give this a rebase and resolve the conflicts?

@chrisvest done :+1:

@netty-bot test this please

Thanks a lot, @franz1981 !


Allow masking events on Login when opening the connection

I have an Asterisk instance that generates a great amount of events. The manager is configured to authorize all events:
read = system,call,log,verbose,agent,user,config,dtmf,reporting,cdr,dialplan
write = system,call,agent,user,config,command,reporting,originate,message

My application connects using the Asterisk Management Interface (AMI) but at startup I don't want to be flooded by all events generated by Asterisk. I want to be able to select which events should be transmitted.
Using the Events command I can decide which events should be transmitted. Unfortunately from the moment I login until the moment I submit this command my application is flooded by events.
A non-documented feature of the login action allows us to set the event mask that we are interested in at Login.
This pull request allows you to pass an array of events:
var namiConfig = {
    host: process.argv[2],
    port: process.argv[3],
    username: process.argv[4],
    secret: process.argv[5],
    events: ['system', 'call']
};

var nami = new namiLib.Nami(namiConfig);

When the events option is omitted Nami does not set the event mask on the login action.
Similar to #13 but less intrusive.

Looks like the same was implemented in the PHP version.


[Feature Request] Support Template URLs

Supporting template URLs should relatively simple to introduce.
Example:
      - name: Nuclei - Vulnerability Scan
        uses: projectdiscovery/nuclei-action@main
        with:
          target: https://example.com
          templates: https://raw.githubusercontent.com/foo/bar/main/custom-templates.yml

In index.js, something like this perhaps:
if (templates) {
  try {
    // URL() constructor will throw if a string is not URL, per https://www.rfc-editor.org/rfc/rfc3986
    new URL(templates);
    params.push(`-turl=${templates}`);
  } catch () {
    params.push(`-t=${templates}`);
  }
}

If you are OK with this, I can submit a PR.

@pbnj-dragon If you want to try and make this work, I think a PR would be great.

@olearycrew - Done. Thank you.


Verify full Unicode support

Should support this already, but should test to make sure.

Test with ☃ and €.
The first is 2-bytes in UTF-8, the second is 3 bytes in UTF-8 and 2 bytes in UTF-16.
The first also doesn't render in many fonts.

Fixed in add7e12ec37cc775a2adaa196a876ae607b45bc5.


Edge of a container visible on right side (iPad)

See screenshot.

Ignore the empty left hand container. That's a different issue.

I propose that this is not part of auth required release unless it has some kind of functional issue associated with it.

Fine by me.
Let me just change from the mobile version of GitHub to desktop because I can't change a label in mobile. Do you hear me, GitHub?!

https://github.com/scottwainstock/pbm-ios/commit/d84d7fa67174941c4781c3cdb75657a1c32e23e9

fixed!


fish shell: error on load_nvm function

Operating system and version:
macOS Ventura  13.1
nvm debug output:

nvm --version: v0.39.1
$TERM_PROGRAM: iTerm.app
$SHELL: /opt/homebrew/bin/fish
$SHLVL: 2
whoami: 'sairoko'
${HOME}: /Users/sairoko
${NVM_DIR}: '${HOME}/.nvm'
${PATH}: /Applications/Sublime Text.app/Contents/SharedSupport/bin:${HOME}/.local/bin:/opt/homebrew/bin:/usr/local/opt/python@3.7/bin:/usr/local/sbin:${HOME}/.pyenv/bin:${HOME}/.poetry/bin:${NVM_DIR}/versions/node/v14.19.0/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/X11/bin:/usr/local/go/bin
$PREFIX: ''
${NPM_CONFIG_PREFIX}: ''
$NVM_NODEJS_ORG_MIRROR: ''
$NVM_IOJS_ORG_MIRROR: ''
shell version: 'fish, version 3.6.0'
uname -a: 'Darwin 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:03:51 PST 2022; root:xnu-8792.61.2~4/RELEASE_ARM64_T6000 arm64'
checksum binary: 'shasum'
OS version: macOS 13.1 22C65
curl: /usr/bin/curl, curl 7.85.0 (x86_64-apple-darwin22.0) libcurl/7.85.0 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.47.0
wget: /opt/homebrew/bin/wget, GNU Wget 1.21.3 hecho en darwin22.1.0.
sed: /usr/bin/sed
cut: /usr/bin/cut
basename: /usr/bin/basename
rm: /bin/rm
mkdir: /bin/mkdir
xargs: /usr/bin/xargs
git: /opt/homebrew/bin/git, git version 2.39.0
grep: /usr/bin/grep, grep (BSD grep, GNU compatible) 2.6.0-FreeBSD
awk: /usr/bin/awk, awk version 20200816
nvm current: system
which node: /opt/homebrew/bin/node
which iojs:
which npm: /opt/homebrew/bin/npm
npm config get prefix: /opt/homebrew
npm root -g: /opt/homebrew/lib/node_modules

nvm ls output:

       v14.19.0
       v18.13.0
->       system
default -> 14.19.0 (-> v14.19.0)
system -> 14.19.0 (-> v14.19.0)
iojs -> N/A (default)
unstable -> N/A (default)
node -> stable (-> v18.13.0) (default)
stable -> 18.13 (-> v18.13.0) (default)
lts/* -> lts/hydrogen (-> v18.13.0)
lts/argon -> v4.9.1 (-> N/A)
lts/boron -> v6.17.1 (-> N/A)
lts/carbon -> v8.17.0 (-> N/A)
lts/dubnium -> v10.24.1 (-> N/A)
lts/erbium -> v12.22.12 (-> N/A)
lts/fermium -> v14.21.2 (-> N/A)
lts/gallium -> v16.19.0 (-> N/A)
lts/hydrogen -> v18.13.0

How did you install nvm?

Yes, I have already installed nvm through the shell command from the readme file
What steps did you perform?
Use the function load_nvm for the fish shell
What happened?
When I navigate between my folders through shell the function load_nvm for the fish shell is invoked, so this function doesn't work correctly when using the following command nvm version because the alias is returned instead of the real version (for example v14.19.0) this provokes that each update in $PWD the nvm use default it's called
What did you expect to happen?
The load_nvm function evaluates correctly the current version
The proposed solution that works for me
# On ~/.config/fish/functions/load_nvm.fish
set -l node_version (nvm version (nvm current))

Is there anything in any of your profile files that modifies the PATH?

If you are having installation issues, or getting "N/A", what does curl -I --compressed -v https://nodejs.org/dist/ print out?

I don't use fish, so I'm not sure what to do here - this was added in #2208 by @jaredramirez. Maybe they can shed some light?

On ~/.config/fish/functions/load_nvm.fish
set -l node_version (nvm version (nvm current))


Update standard workflow files.

This PR was created automatically by the repo_checks.py script in the
https://github.com/openedx/terraform-github repository.

Looks like this is an issue with the commitlint CLI, and currently there is no fix in the github action we're using...
https://github.com/conventional-changelog/commitlint/issues/3256
https://github.com/wagoid/commitlint-github-action/issues/560
I'm gonna drop that workflow from this PR and land the rest, we can leave the PR open for the other one until the upstream issues are resolved.


Play RL replay video as widget

Continuing https://github.com/huggingface/huggingface_hub/pull/659

Loading
Available
Unavailable

checkout the netlify demo here

I love it. I clearly love it. Thanks @mishig25 for this widget, and especially for your reactivity since long story short he was able to prepare everything in less than a day, for the launch of Decision Transformers 👏.

when do you aim for this to be merged @simoninithomas @mishig25?

when do you aim for this to be merged @simoninithomas @mishig25?
I would say the better today. And if not possible on monday. We'll launch Decision Transformer this afternoon (blogpost and tweet, the reddit announcement will be on monday). But we also published the videos on the model card if widget is not ready for production.

BTW has this been deployed to a netlify env?

Should we wait for Decision Transformer to be released in transformers before announcing it? Cc @LysandreJik

@julien-c netlify demo here

maybe the widget is missing a title (for balance and consistency w/ other widgets), no?
Maybe "Hosted Inference widget" or even just the regular title of Hosted inference API  (we don't care if it's not very accurate, no)?
WDYT @gary149

maybe the widget is missing a title (for balance and consistency w/ other widgets), no?
I added "Video Preview" as a title in my last commit and changed a few UI details:

@gary149 thanks so much for the changes! looks beaturiful
Please try the updated netlify here


nothing happens when i click on "PREPARE CARD TERMINAL" in SumUp example app

Why? Device seems to be paired...
Thank you for your feedback! :-)

still the same issue after preparing bluetooth connection to terminal in the demo app...

closed


feat(tokens):  nx tokens build integration

Description
Adding NX support for the tokens build 🏎
What's included?

Build step for tokens to integrate with the other NX build packages for release nx build tokens
GH action to build and commit generated files to libs/token folder for use through out mono-repo
Tokens build package will now be available at @covalent/token in NPM and referenced via cdn https://cdn.jsdelivr.net/npm/@covalent/tokens@latest

Test Steps

[ ] npm nx build tokens
[ ] finally observe the built files generated in the dist folder

General Tests for Every PR

[ ] npm run start still works.
[ ] npm run lint passes.
[ ] npm run stylelint passes.
[ ] npm test passes and code coverage is not lower.
[ ] npm run build still works.

:tada: This PR is included in version 6.1.0 :tada:
The release is available on:

v6.1.0
GitHub release

Your semantic-release bot :package::rocket:


reenable samples tests

I disabled all samples tests in #750. They need to be reenabled with the proper authentication after migration to nodejs-docs-samples.

Closing this issue here, as it's been recreated in https://github.com/GoogleCloudPlatform/nodejs-docs-samples/issues/2963


Add upload ndk symbols

We are working towards allowing notifies from Android NDK.
This change adds code to parse debug information from shared object files within the project, create mapping files and upload the mapping files to Bugsnag to allow de-symbolication of native stack traces.

It seems like there are two directories that we need to keep track of here:
ProjectDir the place where the source files are and the build folder is
rootDir the place where the local.properties file lives, which may be different if the source is in a  sub-project / package

The upload server changes in https://github.com/bugsnag/bugsnag-event-worker/pull/515 should allow for the full file to be uploaded, or the version with redundant addresses removed


Request project link thinks Language Project projects are custom

Fixed in https://github.com/sillsdev/languageforge-lexbox/commit/af033b8d51bb7497eba333e2de8484529d5c4455

This works from the email tester. We don't currently have a way to test this otherwise, because the real request emails go to our super admins.


[CS2103T-W12-2] Trackr

Trackr helps teaching assistants manage the details of their students. It centres around CLI, allowing for quick and easy management of student records via the typing of commands.

Codecov Report

Merging #20 into master will decrease coverage by 8.02%.
The diff coverage is 32.70%.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6389d85...8bac46e. Read the comment docs.


dropdown

nit: make an index.ts it'd make the imports look a bit better.
have you tried the change in core? it looks like it'd work fine. but you'd probably want to take these source files from core instead of saas.

https://github.com/determined-ai/shared-web/pull/17 should fix the issues of the tests failing here