Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

title #5

Open
wants to merge 10,000 commits into
base: vm
Choose a base branch
from
Open

title #5

wants to merge 10,000 commits into from

Conversation

zhangjiayin
Copy link
Owner

No description provided.

David Tolnay and others added 28 commits April 26, 2024 08:05
Summary: This was originally blocked by the presence of `Success` variants holding boxed stream trait objects. No longer an issue since {D56556825}.

Reviewed By: Imxset21, shayne-fletcher

Differential Revision: D56620549

fbshipit-source-id: 4ece683fed9e4f449054d16a7ed4050315849b4f
Summary:
The Thrift generated client code uses *ResponseExn when dealing with streams and *Exn when dealing with non-streams, but also occasionally for streams, converting between the two types.

Prior to {D56556825}, this made sense because the `Success` variant represented a different thing in *ResponseExn vs *Exn. But since `Success` has been removed from these enums, *ResponseExn and *Exn are now identical. Both just hold each of the function's declared exception types and ApplicationException.

This diff deletes *ResponseExn and consistently uses *Exn in both stream and non-stream code.

Reviewed By: Imxset21, shayne-fletcher

Differential Revision: D56620174

fbshipit-source-id: 9f5b04178f782cc3b9c96843abd311b1539b3cc3
Summary: - This move a big chunk of repo global data configs

Reviewed By: ricklavoie

Differential Revision: D55945002

fbshipit-source-id: 7c8161f35fc1a549b4394b595d867197b1c03490
Summary: - Move the last 3 so all hhbc configs are not in configs.specification

Reviewed By: jano

Differential Revision: D55945005

fbshipit-source-id: 779c35557ee6710011182853d2a117c480273fc7
Reviewed By: jano

Differential Revision: D55945001

fbshipit-source-id: 0b957ae2a430ad495f8ac9fab51c5c5f18c28961
Summary:
The LLVM warning `-Wmissing-field-initializers` has found one or more structs in this diff's files which were missing field initializers.

This can be unintended such as:
```
my_struct s1 = {0}; // Initializes *only* the first field to zero; others to default values
my_struct s2 = {}; // Initializes *all* fields to default values (often zero)
```
or it may be because only some of the members of a struct are initialized, perhaps because the items were added to the struct but not every instance of it was updated.

To fix the problem, I've either used `{}` to initialize all fields to default or added appropriate default initializations to the missing fields.

 - If you approve of this diff, please use the "Accept & Ship" button :-)

Reviewed By: palmje

Differential Revision: D56614162

fbshipit-source-id: 3bd6c8b5fc5075f77ab25b622eb6b054d446af33
Summary: Give folly depends on and picks up libdwarf, add a manifest for libdwarf to make it explicit and stop it being found via other means

Reviewed By: markbt

Differential Revision: D56630711

fbshipit-source-id: 7b9386b4b93788e7efda13e51e35c9fec4fd6df2
Summary:
Add support to receiving DC's in thrift servers.
1. Add the extension to the certreq msg
2. Add the custom factory to allow parsing the dc extension from the cert

Reviewed By: knekritz

Differential Revision: D55380489

fbshipit-source-id: 623cc87675f4e968648178a1cde98ec3b948c2c6
Summary:
Actually compute the ideal cursor position for the `override method` refactor.

After this diff, the cursor will be placed in between the brackets!

Reviewed By: mheiber

Differential Revision: D56480226

fbshipit-source-id: f4ea2f0c7aa4132270c4f6848ad8fb0c0516e14b
Summary:
D56480226 introduces logic to move the cursor position to inbetween the braces after an "override method" quickfix.

This diff updates the snapshot tests to render that cursor position as `>||<`. For example:

```
...
  public function foo(): void {>||<}
...
```

If the selection would be a nonempty range, the selected characters would appear between `>|` and `|<` like so:

```
some >|selected|< text
```

but currently nothing in `hh` exhibits such behaviour

Reviewed By: mheiber

Differential Revision: D56570133

fbshipit-source-id: dcd0a1c7a65e5cbcc1b791dbc27dfd7112e2b3d5
Summary:
Instead of inserting:

```
  public function foo(): void {/*cursor here*/}
```

insert

```
  public function foo(): void {
    /* cursor here */
  }
```

It was my hope that code compose would kick in. Unfortunately, it doesn't.

Reviewed By: mheiber

Differential Revision: D56576528

fbshipit-source-id: 4ed380b59fa81b37fd9b6d00229e608f7d8a3994
Summary:
Previously, the Thrift client implementation would deserialize errors as **server** errors, then separately convert the server error into a **client** error. This diff makes the client deserialize directly into the correct client error to begin with.

This diff adds some #[doc(hidden)] types to hold trait impls, but those will become `pub(crate)` in an upcoming diff which moves the definition of client error types into the `-clients` subcrate.

Reviewed By: Imxset21

Differential Revision: D56636985

fbshipit-source-id: ffd59c0b682540088dee5d31d87c8e710b855844
Summary: In preparation for relocating all of errors.mustache into the `-clients` subcrate. Client errors will no longer be exported by the "types" crate which is shared in common with server code, because you only use these client errors when using a Thrift client.

Reviewed By: Imxset21

Differential Revision: D56636984

fbshipit-source-id: 8be0cab9ed6df13b4f46553b1f5989bf80de24d8
Summary:
The point of the analyze constants phase is to fold constants into one
another. So, analyzing functions and their return types, or trying to
inline interp functions is a waste of time. It doesn't really affect
the folded constants, and generates more false dependencies, which
slows down the phase. Avoid all this by not resolving functions or
doing inline interp when we're analyzing constants.

Reviewed By: mdko

Differential Revision: D56619964

fbshipit-source-id: 73a0a6b670dc15e6f4095c9bd924d66df09c0890
…ification results

Summary:
* Thrift connection event "transport.metadata" is being classified into "transport.metadata", "no_transport_verification" and "transport_verification_failure" according to id verification results:
    * transport.metadata -- Identity verification is successful.
    * no_transport_verification -- Identity verification isn't performed.
    * transport_verification_failure -- Identity verification results in mismatches.
* With the change, we can set different sampling rates for those events with different names. We need the change in order to increase the sampling rate for events with id verification error before enforcing the identity verification.

Reviewed By: robertroeser

Differential Revision: D55638974

fbshipit-source-id: 1a7754900918dd0f6f3d4ebb14acfbcc977851af
…n error

Summary: If we specifically allowed missing includes, it should not still be reported as an error.

Reviewed By: iahs

Differential Revision: D56637648

fbshipit-source-id: 1d04777d4a0bbf6bfcb9e475f386b38a3ecc1622
Summary: We can distinguish StructPatch::remove from MapPatch::remove and SetPatch::remove as we over the wire encode as `list`. Use that information to provide better extraction of read and write mask for StructPatch.

Reviewed By: Mizuchi

Differential Revision: D55767979

fbshipit-source-id: 1f0d87531db0cdb65060b8adf28a989e8dc5ae3c
Summary:
This verifies that basic types reflection with thrift-python types works in auto-migrate mode.

Unfortunately, it only works for the subset of types without a container field. We should prioritize generating containers, or keeping them in some limited capacity, because they're going to be a blocker for most modules features fully working

Reviewed By: yoney

Differential Revision: D56601461

fbshipit-source-id: 1e87b2911352580c06e4aae738cd2bec83c4f4d8
Summary: This code is originally from {D28909359}. I don't think we knew about `{{.}}` at the time.

Reviewed By: diliop

Differential Revision: D56643616

fbshipit-source-id: 0b0b4f841a703030ea176ddc4b255016985a1d56
Summary: It can be caused by the client, and thus it is not considered an indicator of something wrong with the server.

Reviewed By: ricklavoie

Differential Revision: D56646094

fbshipit-source-id: d902ec09a384986e8ab24b2d008ee0fac90c6038
Summary: We generally use the term "primitive type" in most of our internal docs so base type here has always confused me, especially since "base type" tends to mean "interface which gets inherited from" but these are actual just basic integral/string types, so I'm renaming base_type -> primitive_type to avoid that confusion.

Reviewed By: iahs

Differential Revision: D56593605

fbshipit-source-id: 445e6fe46c00053fac32fe50e46c92d46ca224a3
Summary: This is effectively part of D56593605 in that it's renaming base_type -> primitive_type (and all other instances of the word "base" in t_primitive_type.h), I just split this out to avoid that diff being even larger.

Reviewed By: iahs

Differential Revision: D56595315

fbshipit-source-id: 16809b7b199ba77f3fecedf8397594eca62a0602
…artup

Summary: Watchman's eden watcher reads a symlink on startup. If Eden is in a non-connected state, this will fail. Recently, we added logic to catch this type of failure and not crash, but it appears that the contraints were too tight. As this is difficult to test or repro, lets try relaxing the constraints and adding some additional logging. Ideally this will stop the crashing along with informing us why we weren't catching it previously.

Reviewed By: quark-zju

Differential Revision: D56642271

fbshipit-source-id: e73ed2b7599c2d62aa1fd441a7a2de7b2fff125f
Reviewed By: zertosh

Differential Revision: D56655848

fbshipit-source-id: 3ff1c47bd562e6fa88e17432f1535dd7002044f5
Summary:
Adds a placeholder for the new IMetricCollector interface.

Callbacks will be added to this interface to be called at specific points/milestones during the lifetime of various server-side Thrift components. This metrics-specific interface will eventually obviate the need to implement metrics instrumentation using the general purpose APIs (e.g. TServerObserver, TProcessorEventHandler, etc) which are not designed specifically for this purpose.

Reviewed By: sazonovkirill

Differential Revision: D56543238

fbshipit-source-id: 84f64c0ede4dc658d6735d6deb985314b97cd356
Summary: Adding a new field useInMemoryTicketSeeds to ThriftServerConfig. When set to true, ticket seeds are stored in memory; false if ticket seeds are read from a file. This field will be used in D55831938.

Reviewed By: AjanthanAsogamoorthy

Differential Revision: D55661709

fbshipit-source-id: e4ec88b334ccaa81d855177ad69ce7061c50c284
Summary: Implement a class `TLSInMemoryTicketProcessor` to initialize and periodically updates random ticket seeds.

Reviewed By: AjanthanAsogamoorthy

Differential Revision: D55667813

fbshipit-source-id: 40d9833f24f8fda1ef20fad1a7b7a6aa183dbe22
ricklavoie and others added 30 commits May 3, 2024 15:37
Summary:
It's clear why we need to decrease the numPops for Dup, but not for
CGetL2. Like the previous diff, this causes failure to reach a fixed
point.

Reviewed By: mdko

Differential Revision: D56909743

fbshipit-source-id: 7ad19bb4259555959f58a9bc346293149ea0937e
Summary:
Prior to this change, requests shed by a static limit (e.g. static maxRequests or maxQps) would understood by the CPU-CC algorithm as if they were caused by CPU-CC. This could cause CPU-CC to incorrectly raise the maxRequests/maxQps (depending on the mode) limit in response.

This change ensures that CPU-CC will only ever adjust the limit if:
1. CPU-CC is enabled, and
2. The load shedding mechanism matches the current CPU-CC operating method (`CONCURRENCY_LIMITS` or `TOKEN_BUCKET`)

Additionally, CPU-CC's `requestShed()` method now returns a boolean indicating whether or not CPU-CC was responsible for shedding the request.

Differential Revision: D56845232

fbshipit-source-id: c6eab3e049c8ae8ed9dadc33f03bf4706ea46212
Reviewed By: joshkehn, prakashgayasen

Differential Revision: D56554114

fbshipit-source-id: 6da2264fce3de119d3180eb2781b9b03b42611f7
Summary:
Now, the `SaplingBackingStore` know the source of each request. However, the sourced is available only for the finish event. The start and queue event doesn't know the source. In the last diff we used UNKNOW for these events. This diff changed the FetchedSource to a std::optional. Therefore for these events we don't need to populate it.
On the other hand, eden.thrift has a NOT_AVAILABLE_YET option. It chooses this option when the std::option doesn't have value.
The kFetchedSource map also update then if we have NOT_AVAILABLE_YET as the fetchedSource, it will give us a blank space character " " and the trace hg will print blank space char for start and queue events.

Reviewed By: kmancini

Differential Revision: D56942308

fbshipit-source-id: 732d5b4fd11cd2e9c5b8ff8e6dc1ec70bc262033
Summary:
This diff solves the problem of extra log spew from Thrift Server in Meta's CLI tools, such as `jk`.

The problem is that CLI tools use CLF, and CLF uses Thrift Server under the hood, as an internal component. In such scenario ThriftServer's startup logs are confusing.
But we do need Thrift Server to print logs in cases when it's used as a web server (that is a typical use case).

This diff adds method `ThriftServer::disableThriftServerLogging()` that sets log level for category `thrift.lib.cpp2.server` to `FATAL`.

**NOTE**, that this only affect Folly Logging log statements (XLOG). Google Logging log statements (LOG) are **not** affected.

In the future we might consider moving all Thrift Server logs to Folly Logging. But for now this diff changes all logging statements in `ThriftServer.h` and `ThriftServer.cpp` to Folly Logging (e.g. LOG() -> XLOG).

Reviewed By: joshkehn, prakashgayasen

Differential Revision: D56554742

fbshipit-source-id: 14e5cca139df0b99dee752673d83829a5d9704d8
Summary: As in title.

Reviewed By: afrind, hanidamlaj

Differential Revision: D56948253

fbshipit-source-id: 4130444cd53e590aaf1e31f2da896d8edb56817d
Summary:
Move profiling to a separate IR instruction.

Makes it possible for later diffs to handle IterInit fully in JIT in most
situations, even if we are emitting a profiling or live translation.

Differential Revision: D56921593

fbshipit-source-id: f7d7eed7b1c4efab1ccfaaba5435e8d24abfed4a
Summary:
follow up on https://www.internalfb.com/diff/D56385248?dst_version_fbid=818286906834949&transaction_fbid=1014920580251512

It seems that leaving the memo key in the m_map seems to yield better perf (could also be measurement noise). It doesn't take much memory so adding it back in

Reviewed By: jano

Differential Revision: D56953255

fbshipit-source-id: 3b827fc893c37bc7f0ed796d09f5f7c208d50cae
Summary:
Keyspace based shadows with bucketization often dont have enough unique keys to exhaust the configured additional_fanout destinations. This diff modifies the additional_fanout logic so that, if used on a PoolRoute with bucketization enabled, will suffix the routing key with the client ip address to increase the fanout across the additional_fanout destinations.

In order to use this the client ip address, its necessary to enable flavor option retain_source_ip.

Reviewed By: alikhtarov

Differential Revision: D55020352

fbshipit-source-id: 1350b80afb06bcf79fd135e5f5158d287ef3f218
Summary:
Allow code of this form:
```
static auto const& vec = std::invoke([] {
  auto& vec = *new folly::small_vector</* ... */>();
  // fill vec ...
  return vec;
});
```

Previously, Leak Sanitizer (LSAN) would observe a leak since the internal data pointer within `small_vector` may be encoded.

Differential Revision: D56890898

fbshipit-source-id: 3cc1110d41a055546f0fa3da1ac2c54de88e9c35
Summary:
/tmp/perf-${PID}.map file has useful content for figuring out which translation
did we crash in and for correlating main, cold and frozen sections of that
translation.

For example, if the last operation was a side exit to the interpreter, it was
likely in the cold or frozen sections and the code usually does not have back
references to the main section and it involves a lot of hunting to figure it
out. The perfmap file can be used to find the main section.

Reviewed By: mdko

Differential Revision: D56982015

fbshipit-source-id: 997555a58a5efaf08dd2adf71241e60a9f2f2d86
Summary:
Serializing Types via BlobEncoder (and decoding via BlobDecoder)
introduces a few issues. It can make a Type "non-canonical" because
the serialized Type reflects the class information the extern-worker
job knew. This class information may be different from another job
knows. Namely if the current job has better class information, it may
have produced a better Type than what the other job produced. This
usually appears with intersection lists. Another job may have produced
a Type which an intersection list, but another job may have had better
class information and thus was able to produce a Type with a single
class.

To fix this, introduce the concept of a "serialized" res::Class. A
serialized res::Class is basically just the name of the class and you
can't do anything with it. If you want to actually use that class, you
must call the unserialize method on it. At that point, it will
actually look up the class and then be usuable. This is usually not
done yourself, but via unserialize_classes in type-system.cpp. This
converts any classes within the Type to not be serialized and brings
the Type to "canonical" form (as far as known class information
permits). Any Type which comes from from BlobDecoder must have
unserialize_classes called on it before being used. This also
registers any dependencies needed, to ensure the Type always
canonicalizes to that or better in later rounds.

Reviewed By: mdko

Differential Revision: D56967225

fbshipit-source-id: 7cc0033c5836088e09b3c99afb2904e77a74aa46
Summary:
When madvising away the local RDS, the code was hardcoding 4KB for the page size,
which isn't always correct (this hardcoded constant has been around since 2014
-- D1545777). This could result in inadvertently madvising away RDS locals,
including `RequestInfo::s_requestInfo`, causing them to be zeroed out for
following accesses.

There was also a bug computing one of the sizes to be madvised, which is also fixed.

Reviewed By: ricklavoie

Differential Revision: D56976784

fbshipit-source-id: 36998891a8380072f425807096aa63b686ac18a5
Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. It's mysterious that this code even still works, but, nevertheless, I'm fixing it.

Differential Revision: D56987484

fbshipit-source-id: e1e641424271e1164eee19be5726ec6abe0c1ffd
…ransport-inl.h

Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. It's mysterious that this code even still works, but, nevertheless, I'm fixing it.

Differential Revision: D56987456

fbshipit-source-id: 4e3fa1e55c43e3c9b72029f83aac1ae15644cf36
Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. It's mysterious that this code even still works, but, nevertheless, I'm fixing it.

Differential Revision: D56987436

fbshipit-source-id: 73c86ace2ef88a26d9f8daa1ea5cdf42d749fea4
Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. It's mysterious that this code even still works, but, nevertheless, I'm fixing it.

Differential Revision: D56987401

fbshipit-source-id: 76593c3efebbd4ea0016b0f0ab1db02aabe706d1
…erator.h

Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. It's mysterious that this code even still works, but, nevertheless, I'm fixing it.

Differential Revision: D56987468

fbshipit-source-id: 98f9aa284b31d275b0586bf691b2e9c5b649e194
Summary:
Treat it like Neg_prim

This is really just a fix for the error message since if all you have is a `not prim` type, you still need a default

Reviewed By: dlreeves

Differential Revision: D56939814

fbshipit-source-id: 2ae6cdda3b887d05c9954decc8dd169ff1dc6d83
Summary: These (https://fburl.com/code/p2hrvjvl) are the only calls that might pass in `void` or `noreturn` and we don't seem to do much interesting with negative types directly on the LHS of a subtype check.

Reviewed By: dlreeves

Differential Revision: D56939927

fbshipit-source-id: b355901b7e7e84b481a340c71723cf79e2598d33
Summary: Earlier diffs removed all constructions of Neg_prim, mostly replacing them with Neg_predicate

Reviewed By: dlreeves

Differential Revision: D56940207

fbshipit-source-id: 27614a01a7518f2a441a2c9fc37348372c64a716
Summary:
The following rules were deshimmed:
```
//folly/experimental:jemalloc_nodump_allocator -> //folly/memory:jemalloc_nodump_allocator
//folly/experimental:jemalloc_huge_page_allocator -> //folly/memory:jemalloc_huge_page_allocator
```

The following headers were deshimmed:
```
folly/experimental/JemallocNodumpAllocator.h -> folly/memory/JemallocNodumpAllocator.h
folly/experimental/JemallocHugePageAllocator.h -> folly/memory/JemallocHugePageAllocator.h
```

This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.

Differential Revision: D56308353

fbshipit-source-id: 4762cf50bf77fe2b10c767e2d8df9a5c77e5d8db
Summary:
The following rules were deshimmed:
```
//folly/experimental:jemalloc_nodump_allocator -> //folly/memory:jemalloc_nodump_allocator
//folly/experimental:jemalloc_huge_page_allocator -> //folly/memory:jemalloc_huge_page_allocator
```

The following headers were deshimmed:
```
folly/experimental/JemallocNodumpAllocator.h -> folly/memory/JemallocNodumpAllocator.h
folly/experimental/JemallocHugePageAllocator.h -> folly/memory/JemallocHugePageAllocator.h
```

This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.

Reviewed By: yfeldblum

Differential Revision: D56308346

fbshipit-source-id: 0c03dc304d8c6ae76675d4dce95b5f75f1b34e4e
Summary: It is missing `rebind` which at least some versions of libstdc++ with g++ seem to require.

Reviewed By: dmm-fb

Differential Revision: D56971559

fbshipit-source-id: 33fe5452702c9a5d385e236035ba0e6a02fbfe47
Summary: C++20 has [eliminated](https://en.cppreference.com/w/cpp/types/result_of) `result_of` in favour of `invoke_result`. This diff updates the included code so that it will continue working with C++20.

Reviewed By: mdko

Differential Revision: D56987624

fbshipit-source-id: 0798539eb02b8da411c5c5b05593c369e23aaee4
Summary:
JIT typesystem doesn't really support Int|PtrToElem. JIT doesn't know how to
encode these values in registers (does it need to distinguish between these
types, like we do for Int|Str?). Core instructions, such as AssertType, refuse
to operate on them.

IterInit/IterNext use Int or PtrToElem types with these instructions based on
what they are iterating. Different specializations of the same loop could use
mix of both types. This becomes problematic when load-elim decides to optimize
away these loads and stores and phis these types into Int|PtrToElem SSATmps.

So far we have avoided this sketchines by pessimizing iteration to a generic
C++ logic whenever multiple specializations would appear, and tracking this
in IRGS in a very fragile way.

There is no good reason why we should not be able to iterate fully from JIT all
the time.

Fix this sketchiness by always using integers and casting as needed. This will
be also useful later for moving iterator state completely away from Iter slots.

Also kill a LdIterPos load-elim optimization that was never used, since
StIterEnd never stored anything more precise than PtrToElem.

Differential Revision: D56971947

fbshipit-source-id: 64cca16a1b02d4f12c694cfdc8e8528e96914705
Summary:
There's a sequence of events that can lead to data being split over multiple QUIC packets when not needed.

If sendHeaders is called from a loop callback, HQ asks QUIC for an onWriteReady callback, and it gets pushed to the end of the currently executing loop callback list.

Consider:

```
loopCallbackA:
 sendHeaders
   schedule onWriteReady in this loop
 schedule resumeProducer in this loop

onWriteReady
 write to QUIC

resumeProducer
 sendBody
```
With this change, we don't ask to schedule onWriteReady until the end of the current list of scheduled work.

Reviewed By: hanidamlaj

Differential Revision: D56955428

fbshipit-source-id: 9d70ad3f733b9950e5e86973e7691d99181eb6a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet