Contract Data: limit key size

We need to decide how to limit the key size: it's a scalability issue in any index (like buckets db), and will cause problems in any of the ledger space reduction approach.
Right now the key and LE are:
...
case CONTRACT_DATA:
    struct
    {
        Hash contractID;
        SCVal key;
    } contractData;
...

struct ContractDataEntry {
    Hash contractID;
    SCVal key;
    SCVal val;
};

I can see a couple ways to mitigate this:

Place some constraints on key, for example, limit its xdr encoded size to be less than 100 bytes. This seems a bit arbitrary.
use a hash for key instead of an SCVal - we can still support host functions that take SCVal, but we can also expose the low level function that takes a Hash. In some cases, contracts will have to store the preimage in val (alongside whatever they need to store), but I suspect this is not that common: contracts don't have a way to list all keys, so contracts always construct keys in some way.

We place constraints on what the topic can be for events. Would the same constraints be sufficiently small for storage keys? If so it's convenient that a developer would only have to think about one type of constrained set of value types.

If people want to have "larger" keys, they just use H(key) as storage key.

I think this is reasonable. It does lose some transparency of what the contract data is. One of the design decisions that @graydon reminded me of recently was that contract is stored structured so that other systems can inspect it, so I wonder if changing the key to be a hash would prevent other systems from doing that. It's obviously still possible for other systems to lookup data if they know what the preimage is because they can generate the hash themselves, but if other systems don't know the preimages and iterating over the stored data, they won't be able to derive the preimage. I would guess contract developers would just need to make sure to duplicate any data other systems might need into the stored value, not the key, which seems fine.

This is related to the limited ScVal discussion, where one proposal is to limit the number of ScObjects (expansion points) an ScVal can contain, and limit the size of a single object. This implicitly limits the ScVal (and thus ContractData) size.

I would guess contract developers would just need to make sure to duplicate any data other systems might need into the stored value, not the key, which seems fine

exactly, I think that the common case is that people don't actually care about this and only a minority of dapps do. We may even be able to cache hashes in the host if this is how we do it and avoid contracts having to deal with it at all.

I think something like a maximum of 8 objects, each with maximum 32-byte leaf-payload size

That doesn't allow for e.g. a BytesN<33>-byte key, does it? My concern about prohibiting using certain data structures well below the size limit still applies here. If you feel really strongly about not using the boundary on the serialized size, then we should at least consider to always allow using a Bytes object up to the max size (say, 248 bytes to account for tags), so that the users could at least serialize their data structures somehow to avoid this limitation (which isn't the perfect UX and isn't most optimal, but provides at least some way to get around the limitations). I guess one could split the serialized bytes into a vector of 8 'chunks', but that seems even less efficient.

I mean, we can design anything we like here, it's a question of simultaneously:

Minimizing chance of surprising user with error
Minimizing set of needlessly-excluded benign keys
Minimizing set of accidentally-allowed trouble keys

Some of that comes down to estimating the set of benign keys likely to occur in practice. I expect all benign keys will have the form of a 32-byte hash, or 25519 key, or symbol, or number, or a vector thereof.
So I think it's probably ok to disallow BytesN<33> and say "you have to hash there" because I don't think users will want to do that very often. But maybe that's too restrictive and we're going to have a lot of 48-byte (384 bit) or 64-byte (512 bit) keys for larger cryptosystems kicking around.
If we expanded the set of allowed benign keys with a rule like "sum of all leaf bytes <= 256, total object count <= 8" my concern is that we'd thereby be increasing the chance of surprising the user with an error (now they have to think about / coordinate the sum of leaf bytes across their key data structure, not just think about individual leaves) but .. not as much as if we forced them to think about serialization size. I could live with either outcome.

I would be ok with the stricter limits, as long as we allow those who can figure out the serialized sizes to bypass the object limitations via just using Bytes. That's probably more relevant for values, than keys, but I guess the policies should be consistent between both keys and values, just with the different limits.

If we expanded the set of allowed benign keys with a rule like "sum of all leaf bytes <= 256, total object count <= 8" my concern is that we'd thereby be increasing the chance of surprising the user with an error (now they have to think about / coordinate the sum of leaf bytes across their key data structure, not just think about individual leaves)

I agree. I think requiring a tighter condition "individual leaf size <= 32 byte" && "total object counts <= 8" is a more clear rule to follow as the user don't need to think of "what's the remaining available bytes to be shared between remaining number of objects". It is also cleaner and preferable from the implementation perspective.
Going from a tighter restriction to a looser one is a much easier one than going the other way (if we decide later this is too restrictive) .
As mentioned above, the majorities of the keys are probably 32 bytes or less. The "escape hatch" always exists to hash the object and use the hash as the key. I don't see an advantage of doing serialization instead of hashing as both forms are opaque and serialized bytes are also no guarantee to fall within the 256 bytes limit.

So here is an updated proposal for limiting contract data ledger key size problem:
We define two new ConfigSettingEntry types:

CONTRACT_DATA_LEDGER_KEY_LIMIT_LEAVES_COUNT. This is the max number of leaf entities a contract data ledger key can have. Initially set to 8.
CONTRACT_DATA_LEDGER_KEY_LIMIT_BYTES_SIZE. This is the max number of bytes a Bytes entity (it is leaf by definition) can have. Initially set to 32.

These limits only apply to ledger keys, not ledger entries. We can add configs for ledger entries as well but this is not within the scope.
(Note: in the future we may add additional “small ScVal” limit config entries for other types: e.g. event topic and event data, if we want to subject them to a different set of limits.)
These limits will be enforced at runtime by the host, i.e. when contract data entries are created or when the footprint is being created. The host will trap/return a HostError during those process if an oversized key has been encountered. This gives the user the immediate feedback when the limit is reached, and is consistent between test and production.
This check is not enforced by the XDR, which means someone may find a way to inject into the Ledger/storage oversized key via other means, but that key will not be compatible with contracts.
Another benefit of enforcing on the host side instead of by XDR is we can have different “small ScVal” types by using the same ScVal with different config entries, rather than having to create new types individually (SmallScValLedgerKey, SmallScValLedgerEntry, SmallScValEventTopic …), or stuck with a one-small-size-fits-all SmallScVal.
The leaf size limit applies to payload not XDR encoded size. So in reality there will be some overhead (if we assume a 4 bytes overhead for each leaf) ⇒ max 32 bytes overhead ⇒ total implicit max size is 288 bytes. This is still small and close enough to the 256 bytes we have targeted.
@dmkozh Does this answer your questions above? @graydon let me know if this matches your understanding.

This check is not enforced by the XDR, which means someone may find a way to inject into the Ledger/storage oversized key via other means, but that key will not be compatible with contracts.

This sounds really concerning. How can one do that if we do the runtime enforcement?

@dmkozh Does this answer your questions above?

Not really, this doesn't answer my question about Symbols and Address and doesn't address my concern about ability to use byte blobs with arbitrary encoding.
Let's take a step back and look at this from the user side, i.e. imagine we need to write a doc explaining the limits to the contract writer. I don't see much point in arguing about the implementation details if we're not on the same page here. Here is what I'd like to see as a policy for any entry stored in the storage:

For every RawVal there is some function GetSize(RawVal)->Size that determines the size of the value. It can be serialized XDR bytes, or a struct with number of objects/sizes etc.
Both storage key and storage value size limits use GetSize to determine if a given RawVal can be written. Key and value have different limits on Size (we can special case keys if you really feel like it... but I'd like to at least understand a reason for doing that). Note, that the encoding format isn't really related to the GetSize policy - if it is recursive, then the runtime limits have to be applied in any case.
If we come up with the count-based restrictions, then we need to be able to explain the GetSize function to the user. I can't claim that I understand the 'leaf entity' terminology. Would it be possible for a user to eye-ball a UDT struct and tell whether it fits into the limits?

This check is not enforced by the XDR, which means someone may find a way to inject into the Ledger/storage oversized key via other means, but that key will not be compatible with contracts.

This sounds really concerning. How can one do that if we do the runtime enforcement?

Sorry if I wasn't clear. My point was, if there was any way for a over the limit key to exist on the ledger -- if it already existed before the limits were placed, or if we have to scale down the limit but have to keep the existing keys, or any other (very unlikely) means to bypass the host to add such a ledger entry. The host will act as the enforcement so it won't be usable as a contract data entry.

@dmkozh Does this answer your questions above?

Not really, this doesn't answer my question about Symbols and Address.

Symbols have a limit of 32 bytes and is a leaf entity so there can be 8 symbols (does matter if each individual symbol is large or small). ScAddress also a 32 bytes leaf entity.
Leaf entity is to disambiguate from Object term since that was causing confusion (a Vec containing 4 empty Maps should count as having 4 entities even if a Map is an Object).

and doesn't address my concern about ability to use byte blobs with arbitrary encoding.

I don't really know what the arbitrary encoding concern means, maybe you mentioned in another thread or channel. Would you mind elaborate?

Would it be possible for a user to eye-ball a UDT struct and tell whether it fits into the limits?

Yes that is precisely the point. With the count approach the user should be able to easily eyeball the structure and tell if it fits or not, without asking the host to decide for them. (we could still provide some host helpers if they want)

if there was any way for an over-the-limit key to exist on the ledger

But there shouldn't be a way to do that (unless we have a bug), right?

we have to scale down the limit but have to keep the existing keys

That's actually an important point - we can't realistically reduce any limits, only increase them.

"Entity" is to disambiguate from Object term since that was causing confusion (

Ok, that sounds better; I've just seen 'leaf objects' term before, which made me think that we need to distinguish between objects and non-objects.

I don't really know what the arbitrary encoding concern means, maybe you mentioned in another thread or channel. Would you mind elaborate?

I've mentioned that above - I suggested allowing to store a flat entry up consisting of up to N bytes or a complex ScVal object that follows the limits that we expect to be about N bytes at most. This is especially important for values (much less so for keys) - the entity number is limited by max_entry_size / max_entity_size and we need to either limit the entity size or the entity count quite severely.

With the count approach the user should be able to easily eyeball the structure and tell if it fits or not

Right, so if we just use 'entities' instead of objects, the things look better - IIUC the number of 'entities' for a UDT struct would be 1 + 2 * number of fields (if fields are flat), unless we flatten the struct into a vector.

if there was any way for an over-the-limit key to exist on the ledger

But there shouldn't be a way to do that (unless we have a bug), right?

Yes pretty much, I can't think of a scenario where this is a wanted behavior.

we have to scale down the limit but have to keep the existing keys

That's actually an important point - we can't realistically reduce any limits, only increase them.

It is unlikely we will need to reduce the limits. However, if for some reason this becomes necessary, say if we miscalculated the impact and someone found a way to exploited it, we would have to migrate the existing over-the-limit contract data to under the new limit (by hashing or some other means). But yes, this is highly undesirable.

I don't really know what the arbitrary encoding concern means, maybe you mentioned in another thread or channel. Would you mind elaborate?

I've mentioned that above - I suggested allowing to store a flat entry up consisting of up to N bytes or a complex ScVal object that follows the limits that we expect to be about N bytes at most.

I see. I thought "arbitrary encoding" meant considering alternative encoding schemes to XDR. This is clear now.
Yeah this is the main difference in our opinions. I still feel the "max number of capped-size entities" approach is easier to follow (and implement). It makes more sense for LedgerKeys, since they should consist of small entities vast majority of the time. For contract data LedgerEntry, I don't think there should be any substantial limit (besides the "circuit breaker" kind of absolute limits). Keys and entries are fundamentally different. We need the size-capped LedgerKey for scalability reasons (e.g. BucketList index building). However, it is plausible the user want to store large data in an entry (e.g. serialized image or something) and if need for a limit do arise, the rules would likely be different (maybe the entity count is no longer relavent and we just want a cap on bytes size, idk). But we will come to that when that need arises.
Also switching from "individually size-capped entities" model to a "share-capped" model is much more doable (than the other way), since we are loosening the limits.

With the count approach the user should be able to easily eyeball the structure and tell if it fits or not

Right, so if we just use 'entities' instead of objects, the things look better - IIUC the number of 'entities' for a UDT struct would be 1 + 2 * number of fields (if fields are flat), unless we flatten the struct into a vector.

Yes! The way you've put it made me realize we need to clean up the terminology further by removing the restriction of "leaf". In the example figure below, the count of entities is 11, including all (leaf and non-leaf) nodes. If we only count leaf node then it becomes more ambiguous: should we count the ('d': Vec) altogether, or should we only count its key? I think we should go with "entity count". In that case, I propose to raise the entity count limit from 8 -> 16. Since a map with len=4 will already have 9 entities.

I am not sure where this thread is going but what @dmkozh wrote above makes sense to me.
So is the summary here that:

at the protocol layer, enforce limits via some xdrsize function (xdr encoded size). This is simple to implement correctly. The same function can be used on any type and in all sorts of situations (such as when storing data, crafting payloads to be used in other chains, etc)
in docs, we can provide some guidance for people to "eye ball" sizes based on entities.

@MonsieurNicolas no, this whole conversation is about avoiding reliance on xdr sizes, because the user can't predict them.

yes, I am questioning this notion that we need to introduce new concepts at the protocol layer.
Every new concept introduced means having to build all sorts of checks & conversion machinery (both within a protocol version and across protocol versions) as well as different testing patterns to ensure that people don't hit those limits.
Not to mention that contract developers too are going to have to project whatever type they're using into that other type space.
As far as keys are concerned, the idea that people are going to just eyeball things and be fine seems fairly optimistic to me: if people are using complex objects as keys and don't write tests, regardless of how "size" is defined it's not going to end very well.

Ok, you're rejecting the course of the conversation then, not summarizing it.
I disagree that doing this in terms of XDR serialization size is safe or wise. It places XDR serialization measurements in code paths that currently don't even have them -- we do not serialize anything when we emit an event or place an entry in a map -- and as we've been discussing it makes it more likely that users have inadequate test coverage, because they have to cover not just the boundary conditions of data structures they are manipulating, but also the boundary conditions of those values as serialized in XDR.

@MonsieurNicolas Here is a summary of my proposal (tweaked from above with less rationale for succinctness):

We define two new ConfigSettingEntry types:

CONTRACT_DATA_LEDGER_KEY_LIMIT_ENTITY_COUNT. This is the max number of entities an ScVal (key) can have. Initially set to 16.
CONTRACT_DATA_LEDGER_KEY_LIMIT_LEAF_SIZE. This is the max size of a single leaf. Initially set to 32. (This is essentially a limit on "bytes" size, since all other val types already conforms to this). This number is the actual "pay load" size, does not include XDR overhead.

These limits only apply to LedgerKey, not LedgerEntry. (We will have a "catch all" LedgerEntry size limit, see https://github.com/stellar/rs-soroban-env/issues/716, but no more granularities than that.)

These limits will be enforced at runtime by the host when 1. contract data entries are created 2. the footprint is being created. The host will error during those process if an oversized key has been encountered.

I disagree that doing this in terms of XDR serialization size is safe or wise. It places XDR serialization measurements in code paths that currently don't even have them -- we do not serialize anything when we emit an event or place an entry in a map -- and as we've been discussing it makes it more likely that users have inadequate test coverage, because they have to cover not just the boundary conditions of data structures they are manipulating, but also the boundary conditions of those values as serialized in XDR.

I concur with @graydon on not relying on the XDR serialization size.

at the protocol layer, enforce limits via some xdrsize function (xdr encoded size) ... The same function can be used on any type and in all sorts of situations (such as when storing data, crafting payloads to be used in other chains, etc)

We already have events which has a different set of rules (no Bytes included in the topic, Vec size <= 4 etc). So I don't think it is productive to enforced a single set of "XDR size" rule on different situations.
We would like to design rules such that most users don't need to think about them. If we have the "max_entity_count" and "leaf_size" set to reasonable numbers, most UDT keys should work without thinking about it. Same goes for the "max contract size" rule, or the "event" rules. However for any outliner, such rules need to be followed closely, and "XDR size" is the far more difficult one to follow.

Keys and entries are fundamentally different.

This is not true. Both are ScVals at the protocol level and RawVals at the host level. See my above comment - I'm asking for a unified definition of GetSize function with different limits for keys and values. There has to be some size limit for both and I would rather prefer to at least have one methodology defined for both. Or are you proposing to go for the counting-based approach for the keys and XDR-based approach for the values? I feel like that would bring even more confusion to the users. Again, I'm not arguing the limits should be the same, but the methodology has to be the same IMO.

However, it is plausible the user want to store large data in an entry (e.g. serialized image or something)

Which is why my proposal is to enhance the count-based approach with the following rule:

If ScVal is Bytes, then allow it to be up to N bytes (where N is the ballpark estimate on max serialized ScVal we allow)
Otherwise, apply count-based limits.

That's it, I'm not proposing anything overly complex, just a simple way to store ScVals with arbitrary structure.

First off, everything has an implicit limit, not only limited by metering, but also by XDR. All LedgerEntry will have an implicit "stop all" limit of around 64kB -- that is the contract size limit. https://github.com/stellar/rs-soroban-env/issues/716 will make that upper limit explicit. But that limit should not be relevant for this conversation.
So to answer your first question, yes I propose a count-based limit for LedgerKey and "no" limit for LedgerEntry. (I'm putting "no" in quotes because again, we are not considering the "stop all" limit)
In terms of your proposal. If we were to adopt the count-based proposal -- placing a limit on max number of entities (of say 16) -- I think it will be more confusing to allow bytes limit to be shared among the entities. E.g. if a Bytes takes up 488 bytes, do we just allow 1 more entity (to reach N=512 bytes), therefore not respecting the max count limit. Or if we still allow 15 more entities, but they cannot be Bytes (and bytes-equivalent types String, ContractExecutable) in which case N will still be exceeded?
I think the only sensible solution here -- if we really need a larger Bytes in the LedgerKey -- is just to increase the "bytes limit" all together while respecting the "entity count", which is same as what I proposed above).

So to answer your first question, yes I propose a count-based limit for LedgerKey and "no" limit for LedgerEntry. (I'm putting "no" in quotes because again, we are not considering the "stop all" limit)

This is where I have a hard time following this thread - why do we think that keys need the extra policy when we would not have that for values?
Of the two, bad keys are the ones that are most likely to be caught by any test because the 80% case (probably more, this is the 80/20 rule) is that keys have a static structure, where as values are likely to contain dynamic data structures like maps and vectors.

ledger keys impact the memory overhead of indexing the bucket list, for starters. they also impact the total transaction size in the sense that they must all be listed explicitly in the footprint.
they are also (like simple map keys) repeatedly compared at runtime using a total-ordering function that is rarely if ever applied to values, so large keys will much more likely exhaust the CPU budget anyways.

All LedgerEntry will have an implicit "stop all" limit of around 64kB

So this is basically the limit on the serialized ScVal stored in the value entry, right? Isn't that what we wanted to avoid by proposing the counting approach?

E.g. if a Bytes takes up 488 bytes, do we just allow 1 more entity (to reach N=512 bytes)

The relationship is opposite. Say, your ballpark count-based limit is 256 bytes. Then just allow having a single bytes ScVal up to 256 bytes. If the root ScVal is not Bytes, count-based limit is applied.

is just to increase the "bytes limit" all together while respecting the "entity count"

This will inflate the maximum size limit, which is not desirable.

ledger keys impact the memory overhead of indexing the bucket list, for starters.

This addresses why we should have a small size limit on the ledger keys, which makes perfect sense.
What doesn't make much sense to me (and I think Nicolas as well) is the way we define the policy.
To reiterate: from the user perspective there is no difference between 'keys' and 'values' until they are put into storage- they are the same data types with the same encoding and behavior rules etc. We need to be able to clearly communicate the size limits to the user to let them design their storage structure, limit user input sizes etc. If we think that XDR-based limits are hard to interpret, we should avoid them consistently. Otherwise, we introduce two different ways of estimating the ScVal sizes and the users have to be aware of them anyway (whether or not we want the users to not hit the limit most of the time is beyond the point; some users will be hitting the limit anyway and we need to be able to provide a clear explanation of why this happens).

Different limits will impact users at different levels of the system. The XDR definitions themselves impose various fixed limits, packetization by the overlay imposes a different set of limits, there are runtime limits imposed by the user's budget (also hard to guess), etc. etc. There's never going to be a "single limit" scenario, that's setting up a false goal.
The goal here is simply to have a barrier that prevents misunderstandings that cause performance problems. It doesn't need to be perfect, it's not going to eliminate or unify all other limits in the system, it just needs to be simple to calculate, simple to explain, and ideally something users are likely to notice during testing rather than overlooking.

I think our first order of business is to decide:  should there be a limit on contract data LedgerEntry?
My understanding was, no.  What @graydon explained is the basis of my opinion.
But if LedgerEntry needs a real (not a "catch all" type of) limit, then the premise of the conversation becomes very different.
LedgerEntry and LedgerKey are functionally different (explained above). Sure one can argue everything is an ScVal in the end so they are all the same, but I don't think this is a useful argument.
Count-based limits can work very well for LedgerKey because it is small, so one can eyeball and tell if entity count is <=16.  That is different for LedgerEntry if such a limit is large e.g. entity count <= 1000.  So should we apply the counted-limited ScVal approach for all types? I don't think that's productive.
Also we have Events in the system that are subject to different set of nuanced limits. Sure it is fundamentally an ScVal too but the "limited ScVal" approach won't help there.

It doesn't need to be perfect, it's not going to eliminate or unify all other limits in the system, it just needs to be simple to calculate, simple to explain, and ideally something users are likely to notice during testing rather than overlooking.

I think we are on the same page here. What I'm still trying to figure out is the explanation part. I just want to see some rough sketch of the documentation section that describes the storage limits.

That is different for LedgerEntry if such a limit is large e.g. entity count <= 1000

I don't really see a difference here, these are just numbers. I want to be able to answer questions like 'can I store 32K byte blob?' or 'can I store a vector of 4000 u64 values?' (the common programming knowledge would tell me that the answers should always be equivalent, but I suspect they're not). Again, we need to document this somehow. If we struggle with explaining the limits to ourselves, then we'll definitely struggle with explaining them to the users. And if we can come up with some framework for explanation, then we can just specify it once and refer to it when talking about every use case. Note, that it's ok if some limits don't apply to some cases - there is nothing wrong about saying something like 'entry - entity count - unlimited'.

I think our first order of business is to decide: should there be a limit on contract data LedgerEntry?

My understanding was, no.

So is your suggestion to only rely on the fees here? I'm not sure if that's practical - are we sure all the systems are capable of handling ledger entries that are multiple MB in size?

99.9%+ users won't have to realistically worry about that limit besides using common sense

This statement needs some basis; I wouldn't be so sure about the user needs. Even if it was true, it doesn't justify hand-waving the explanation with 'no-one needs it' sentiment. It is easy to serialize ScVal and find out the byte size and I'm ok about using that to explain the storage limitations to the user. I thought there are concerns about expressing the limit this way being hard to explain/estimate, but if decide that these concerns are insignificant, I would be ok with that too (which means that I would be also ok with using the same approach for the key size limit as well, but if you're feeling strongly about keys having an alternative limiting mechanism, I guess I'd be fine with it as well. I'm not sure if having two alternative definitions is necessarily simpler, but I also don't think that's this makes enough difference in the end).

I think we're somewhat talking past each other. Let me try again.
There are different limits imposed at different levels of the stack. At least the following layers exist:

IO buffer limits. Due to the way xdrpp works we have to buffer in memory any XDR value we're reading before we decode it. We also have and/or expect buffer-size limits to arise from networking and bucket IO, to control granularity of work and to prevent DoS attacks.
Budget limits. While users will be able to buy some additional CPU time and memory for their budget, we'll also (again for DoS prevention and to ensure sufficient granularity) likely have a lower-level cap that users can't exceed by "paying more".
"Efficiency-encouragement" limits. These are what we're talking about in this bug. They're present not necessarily to prevent a DoS, but to encourage use of the system in a way that's efficient and "works with its grain". Because users are often interacting with a system through a highly abstract lens, they can accidentally treat all resources as "if it works, it's good enough" and this can (and very often does) cause fairly significant performance problems down the line when they start doing "a thing that's wrong-headed from an efficiency perspective, but kinda works" at scale. We'd prefer to stop them before they get started doing such things.

This bug is not about any of the lower level limits. It is only about the last (highest-level) limit in that arrangement. I want to observe now (as we all did back in August when we started discussing this) the following facts salient to that highest-level limit:

Higher-level limits can't really exceed lower-level limits, by construction. If we say an object can be 100 MiB at the high level and then fail when writing it to disk because there's a 256KiB buffer limit, that lower level limit was actually still enforced, it won. Similarly if we say the user has a 16MiB hard memory limit there's no sense talking about an efficiency-encouragement limit on host objects of 100 MiB. At most zero of those objects can ever happen anyways.
Different limits take place in different domains of discourse. At a high level a user is interacting with "objects and values" which have an existence in a programming model in terms of variables, data structures, and arguments passed to functions. At the lower levels the system is thinking about memory bytes allocated, CPU instructions expended, the size of byte buffers, or at most reasoning about XDR padding and structure layout rules, transport and cryptographic framing, and similar much-lower-level details.
The system is structured around separated (bulk) IO and computation, which means violations of a lower-level limit will only be signalled potentially quite a while after the cause of the violation, unless we artificially hoist XDR-ification passes up to operations that currently don't need them. The same violation exceeding a higher-level limit can be signalled more immediately.

If we can agree on those facts, I argue that the proposed plan ("max N objects, each of max size K") falls out as a fairly natural consequence. If we want to augment it with @dmkozh's refinement of "or, at the user's option, exactly one Bytes object of size N*K" I think this is fine, I don't think that's too hard to implement nor hard to explain to users -- it's certainly easier than serializing to XDR and having to explain to users why they have to round up all the strings in their object to multiples of 4 or why there are extra 4-byte tags and size values scattered through their structure. And it's still more efficient.
Finally: this conversation is the very definition of a bikeshed. It's an easy-to-change, low-consequence subject that's somehow managed to draw in multiple senior engineers for multiple days (over multiple bugs, for weeks-to-months). Please just let @jayz22 get something merged that meets the basic need to discourage users from slamming their hands in the door by storing 100KiB blobs in maps, event topics or ledger keys; if we get user feedback that the cure is worse than the disease it'll be very easy to change (raise soft limits, change what's being counted) before we go final.

Please just let @jayz22 get something merged that meets the basic need to discourage users from slamming their hands in the door by storing 100KiB blobs in maps, event topics or ledger keys; if we get user feedback that the cure is worse than the disease it'll be very easy to change (raise soft limits, change what's being counted) before we go final.

I think everybody on this thread agrees to having what you called IO limits above. So we should get this done as they're simple protocol invariants and can easily be implemented on the C++ side for now:
we just need to make sure that if a transaction is about to violate that invariant, we emit some diagnostic event and fail the transaction.

No, this bug is not about the IO limits. Those are already baked into the system. This thread is about the higher level limits.

I thought the low level limit that you were talking about was how big we let a ContractDataEntry get (similar to the limit getting added to ContractCodeEntry in an upcoming PR), which is not implemented today as far as I can tell.
When I opened this bug my intent was to get all those low level limits implemented in env as it's the minimum bar, and walk our way up the stack (literally: those higher level limits maybe just belong in the SDK or custom types implemented on top).

Well, ok, you opened the bug so I can't tell you what you were thinking. I apologize for putting words in your mouth. I read the bug (talking about key size and "100 bytes") as though you're talking about high level semantic limits. The size of a ContractDataEntry is presumably "quite big", like on the order of our lower level limits, hundreds of KiB or a MiB or something (we do have baked-in lower-level limits currently: the overlay won't send a packet larger than 16 MiB, and we have a budget cap on the host of 50 MiB of memory). Maybe you're only talking about the size of the associated LedgerKey that gets extracted from a ContractDataEntry?
What I thought (or hoped) this bug was about was high level limits on the not-yet-serialized, in-memory host representation of a contract data key. Which is domain-specific: we'd reject large keys when inserting into a map, for example, or when publishing an event. Even though all RawVals have in-memory identical size (8 bytes). We have discussed at length in many bugs and many in person and chat conversations the idea of having a single reused "small value type" to use for contract data keys and map keys and event topics, to guide users towards efficient use and not cause explosions of inefficient data-emission downstream when serializing output. This is what @jayz22 mentioned way up above. Same subject.
And I guess my point is that if we do enforce a tight high level limit for "small values" in the host value domain, and it applies to ledger keys, that will automatically imply a correct-ballpark efficiency limit of XDR LedgerKeys we're throwing at the bucketlist, so we might not need any other artificial lower-level limit for efficiency, just IO buffering (currently unlimited on the BL but there's code to support limiting it) and DoS resistance. Note though that any IO buffer limits have no idea what they're reading, and when reading an LE they're reading the whole LE, not just the key part. So the size limits will be completely different orders of magnitude at that level.
IMO if we're intending to have a small-value limit up in the host value domain, we should just get on with it and have one. Currently our treatment of size limits is a bit of a hodge podge: events get one limit and various XDR SCVal types currently have a different one baked in that .. doesn't really work to limit IO load (which was its point) since SCVals can nest and there's no fanout limit specified.

Please just let @jayz22 get something merged that meets the basic need

Sorry, I didn't mean to block the actual changes. The only feature I've asked about is to allow storing the bytes blobs. If we think it's fine, let's just do it. Otherwise, my main concern is about the documentation/communication - it doesn't have to block the implementation. Let's just have this clearly documented in the PR with the changes and the SDK storage interface.

All good, I appreciated the inputs.
I was mainly trying to explain why LedgerEntry would not be considered a "small value" since I thought that was the premise of this issue (and apparently didn't do a great job so thanks @graydon for the thorough explanation above).
I'll try to implement in a way that's easier to adapt to the "shared blob size" approach. I agree it would be a necessary enhancement if we are considering larger sized values.

OK so I think I understand better now where the disconnect came from.
Here is my summary of where I think we have interesting discussion points.
Enforcing limits and invariants in subsystems
(original intent - we may want to move this to different issues given that this issue has been mostly discussing something else).
When I opened the issue I thought that we were going to build limits bottoms up that way we can have clean invariants for each layer and component and be sure that we don't have gaps in coverage.
At the time, for this issue, I was only thinking about the interface to the bucket list as a key value store.
The check you linked to in the BL is not really necessary in that context if we assume that no garbage was written into it in the first place and that the bucket hash was verified before parsing it.
Writes are a different story obviously.
From a security point of view, I think that it's unsafe to not implement limits at key low level boundaries. The higher we get in the stack, the more likely we're going to have bugs and therefore open up backdoors.
Specifically for the bucket list API:
We should be enforcing that any LedgerKey is smaller than 300 bytes and that any LedgerEntry is smaller than 100KB (or whatever network settings get picked).
Higher level developer experience
(what this thread morphed into)
I am not opposed to adding different concepts and limits higher in the stack if it helps improve the developer experience, that said right now I am not sure which problem we are really trying to solve and if it really moves the needle for developers overall.
The use cases around keys seems to be a little too special to help move the conversation forward, which is why I would like to see a proposal that also covers how we're going to deal with values, but maybe I am wrong.
I would recommend reviewing the fee CAP to understand some of the background on this.
I think users already need to think about the following when dealing with objects:

Object layout in their contract as every time a function modifies an object it potentially can cause the runtime to abort because of the limit being hit on the memory utilization front. I think in this context we are really trying to limit number of bytes allocated by the runtime.
How big ledger entries are going to be when reading and writing both to ensure that they are not going to hit the upper limits as mentioned above, but also to optimize cost in terms of fees (rent) data directly derived from the "on disk" (so xdr encoded size).
How big their transaction is going to be as they pass in parameters to the public methods over their contract (propagation and storage fee in archives). Again this is going to be Directly linked to the XDR encoded size of the objects.

if we were to add a new concept such as a size that uses entity count it looks like it would be in addition to having to think about objects in terms of raw byte size, therefore making the developer experience more complicated.
At the same time if the gains are significant I can see why we would want to do something like this. Maybe we should just meet and discuss live to help unblock this work.

@MonsieurNicolas @graydon @dmkozh and I have met and discussed on this issue, and we have decided to take the direction of "imposing a serialized bytes limit, for both LedgerKey and LedgerEntry", concretely:

There will be two new ConfigSettingEntry types defining the max number of bytes a XDR-serialized LedgerKey and LedgerEntry of contract data can take, the initial limits will be around 100 bytes and 100KiB respectively. They are configurable network settings.
These limits will be enforced at the boundary between the stellar-core and the host when the storage data are being converted between host XDR values and serialized bytes, in both the preflight and the actual run.

The the main benefit in this approach is consistency which allows these limits to be interoperable with other limits (transaction read/write limits, bucketlist size limits etc and thus for calculating fees. See fee cap for details). Also it is easier to document and explain and do not require the user to learn any new concepts.
The cons of this approach is mainly the nontrivial mental translation between values and limits (requiring the user to know the XDR nuances to be able to derive object bytes size), discussed several times above. But hopefully this won't be a main issue. We can also provide host functions to assist with getting the limits.

Resolved by https://github.com/stellar/stellar-core/pull/3731.


Initial commit for diffusion problem - fortran version

See OpenCMISS-Examples/burgers_static#3  comments there apply here also.


property: use a stack to efficiently convert index to string

The existing code does this conversion by searching the hash table for the appropriate index which is slow and expensive.  Also add some low level unit tests for the ossl_property_name_str and ossl_property_value_str functions.
Fixes #15867

[ ] documentation is added or updated
[x] tests are added or updated

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests.  Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

Merged to both.
I intended to add some small memory conditionals but will now do so in a different PR.

Merged to both.
I intended to add some small memory conditionals but will now do so in a different PR.

Merged to both.
I intended to add some small memory conditionals but will now do so in a different PR.

@paulidale but this was approved for master branch only as this is a feature. IMO this should be reverted on 3.0 branch or an explicit exception has to be given by OTC.

Bugger, you are correct.  This needs to be reverted.

Revert in #17403


Retrieve threads via API

Feature request
Currently, the LangSmith API provides methods to retrieve runs and their metadata, but does not include the thread_id parameter. I would be useful to include the thread_id in the run metadata. Furthermore, it would be useful to have a thread route on the LangSmith API, in order to retrieve collections of runs, grouped by thread_id.
Motivation
I want to build a system that allows users to clone threads and share them with other users, to continue working with on their own account.

Thanks for writing in! And good question.
A thread's thread_id IS metadata.
You can fetch from the SDK/API by specifying the thread_id you want in metadata. See the doc on exporting traces for some more info: link
This is how the UI itself works :)


Update LUA dependency for hawck-git AUR package

Hi,
The installation of hawck-git fails due to the dependency on lua 5.3 while the current one seems to be 5.4.1.
If no breaking changes, would it be possible to update the build definition? Thanks!

Fixed, thanks for the report.


parse_esri_json() Cannot parse empty featureset

Maybe it could be useful that if there are no matches when using the filter_geom argument in the arc_read function, instead of an error, I get an empty dataset, just like when you do a filter in sf package with the query argument.
library(arcgislayers)

geojson <- '{
  "type": "FeatureCollection",
  "features": [
    {
      "type": "Feature",
      "properties": {},
      "geometry": {
        "coordinates": [
          [
            [
              -72.16260961974183,
              4.170524735334581
            ],
            [
              -72.16260961974183,
              3.5284018431919435
            ],
            [
              -71.25683279344358,
              3.5284018431919435
            ],
            [
              -71.25683279344358,
              4.170524735334581
            ],
            [
              -72.16260961974183,
              4.170524735334581
            ]
          ]
        ],
        "type": "Polygon"
      }
    }
  ]
}'

bbox <- geojsonsf::geojson_sf(geojson) |> sf::st_bbox()
url <- 'https://ags.esri.co/arcgis/rest/services/DatosAbiertos/PNN_2005/MapServer/0/'
data <- arc_read(url), filter_geom = bbox)

When I run that code, I get this error: Error in fts_raw[["attributes"]] : subíndice fuera de  los límites

closed by https://github.com/R-ArcGIS/arcgisutils/pull/21


Classloader issues with devtools

Following on #13, after declaring my session attribute Serializable, a class cast exception like this one occurs:
java.lang.ClassCastException: br.com.mercur.rel.model.Usuario cannot be cast to br.com.mercur.rel.model.Usuario

That doesn't maked any sense at first, class cast exception trying to cast to the same class. Searching a little bit, i got here: https://github.com/spring-projects/spring-boot/issues/3805. From the issue description:

This means that the value restored from session has a class defined by the system ClassLoader. The application itself loads the same class from Spring Boot's RestartClassLoader. Since the two classes are loaded from different ClassLoaders they cannot be cast from one to the other.

The docs from spring boot devtools have a section on this: https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#using-boot-devtools-known-restart-limitations
Looking at the code, the default constructor on JdkMongoSessionConverter uses the serializer/deserializer from Spring, wich seems to be the recommended one from the docs.
Removing spring-boot-dev-tools from the classpath solves the problem. Is there any possibility to keep using dev-tools? It's features are a much appreciated help at developing.

@creusar You might be able to test this out faster than I can. Does setting a serialVersionUID solve this issue?
If you put that value into the class, would the classloader decide the classes are identical, and operate?
See https://stackoverflow.com/questions/285793/what-is-a-serialversionuid-and-why-should-i-use-it

In fact, if this solves the problem, I will augment the reference docs to recommend such a solution in this scenario.

Yes, would be easy to test it. As soon as I do it, would come back and leave a reply.

Doesn't solve the issue. It's all fine when the login page is displayed, but the exception is thrown when trying to save the new session with the added principal (the class with the serialVersionUID).

Does both the class stored and the new class both possess the serialVersionUID?

The session store was empty for this test.

My internal contacts tell me serialVersionUID doesn't help with this, and that rolling through classloaders makes them different classes. So best bet is for me to solve the issues with the Jackson serializer and make that available.

@gregturn do you have any news on this?

Latest release has Jackson support. I’d give that a shot.

The problem is that JdkMongoSessionConverter is using a DeserializingConverter with the wrong class loader. It should be using the same class loader as is used by the BeanFactory to load bean classes. This can be injected into a bean by implementing BeanClassLoaderAware. Please see this issue and the fix for inspiration.

@gregturn Thanks for the fix! :smile:


error in aft-workshop-sample

when executing step4 (Execute the bash script to launch the Cloud9. Change the placeholder {{CT-HOME-REGION}} with your AWS Control Tower home region (AWS region where AWS Control Tower is deployed).) In the prerequisites section of aft-workshop-sample, the following error occurs.
│ Error: Missing required argument
│
│   on main.tf line 34, in resource "aws_cloud9_environment_ec2" "cloud9-aft":
│   34: resource "aws_cloud9_environment_ec2" "cloud9-aft" {
│
│ The argument "image_id" is required, but no definition was found.

Encountered the same issue :|
They must of deleted the image.

The issue is in main.tf
Add a new line 38 with the following
image_id      = "amazonlinux-2023-x86_64"
And it will run as expected.  This was a new property added to the API call at the end of last year.

PR #17 should resolve this now, let us know if you are still encountering problem , thank you.


[BUILD] Add pyproject.toml

Summary

added pyproject.toml

Testing Done

ran pip install -e . and it built successfully

Hardware Type: RTX 3090
[x] run make test to ensure correctness
[x] run make checkstyle to ensure code style
[x] run make test-convergence to ensure convergence

lgtm if we bump to the new ver

@AndreSlavescu let's get this in sometimes


Multiple locales feature is not mentioned

It is not clear if the batch transcription supports multiple languages or locales like the SDK.
Please mention if it is supported or not. If not supported, then possibility of it being supported in the future releases should be mentioned.

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

ID: 73fb7e76-fb80-c420-2efe-e3ca3a27b749
Version Independent ID: 07a42557-deb3-e4b3-3f14-f26a27be49c2
Content: How to use batch transcription - Speech service - Azure Cognitive Services
Content Source: articles/cognitive-services/Speech-Service/batch-transcription.md
Service: cognitive-services
Sub-service: speech-service
GitHub Login: @eric-urban
Microsoft Alias: eur

@karthikbhat6 The batch transcription API supports multiple locales. You can retrieve the list using the GetSupportedLocalesForTranscriptions API with a GET call.
This is mentioned in the batch transcription operation table at the top of this page. I hope this helps!!

@RohitMungi-MSFT I wanted to send more than 1 locale in a single API. Is that possible?

Will that feature be added in the near future?
It is an important functionality and will be very helpful if it is added.

You can submit a feedback item to through Azure feedback forum for cognitive services. Please try to add details about your use case so the service team could review and add the feature for the future releases. Thanks!!

#please-close


[BUG] Connection Closed when sending message to groups with many participants

When trying to send any messages to groups with many participants, the request takes a long time and returns this error. how can i resolve this?
Error: Connection Closed 0|api  |     at sendRawMessage (/home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Socket/socket.js:56:19) 0|api  |     at sendNode (/home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Socket/socket.js:75:16) 0|api  |     at /home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Socket/messages-send.js:436:19 0|api  |     at async Object.transaction (/home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Utils/auth-utils.js:136:26) 0|api  |     at async relayMessage (/home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Socket/messages-send.js:278:9) 0|api  |     at async Object.sendMessage (/home/whatsplan-sender/htdocs/sender.whatsplan.com.br/node_modules/@whiskeysockets/baileys/lib/Socket/messages-send.js:646:17) { 0|api  |   data: null, 0|api  |   isBoom: true, 0|api  |   isServer: false, 0|api  |   output: { 0|api  |     statusCode: 428, 0|api  |     payload: { 0|api  |       statusCode: 428, 0|api  |       error: 'Precondition Required', 0|api  |       message: 'Connection Closed' 0|api  |     }, 0|api  |     headers: {} 0|api  |   }

Use defaultQueryTimeoutMs: undefined in main config in makeWASocket

Use defaultQueryTimeoutMs: undefined in main config in makeWASocket

apartment worked, thank you!

@DavidsonGomes is it still working? Here it stopped, and I made this adjustment and it's not working.
Can you pass your package.json so I can compare the versions?


Job doesn't complete if it can't find the correct number of wells when auto-cropping

The design of the auto_crop function in crop_wells.py is such that an error is raised (and the job crashes) if it can't find the exact number of wells provided in the YAML (multi-well-row/col). An example of the error is attached.
Log files
20220729-p04-KTR.txt
20220729-p04-KTR_10070010_0.txt

Solution: warn the user and then switch to grid_crop. Also, provide better video preparation documentation to help the performance of auto_grid.


branch-3.0: Fix Fix potential data leak when a partial update load which has publish conflict fails #45626

pick https://github.com/apache/doris/pull/45626

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.
Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

run buildall

run cloud_p0


Ruby: Service tests create global classes that need to be namespaced

Currently, the generated service client tests for Ruby generate a few global classes. For example, I just generated video_intelligence, and the generated service client test file test/google/cloud/video_intelligence/v1/video_intelligence_service_client_test.rb includes the following toplevel classes:

CustomTestError
MockGrpcClientStub
MockVideoIntelligenceServiceCredentials

The problem is when we generate more than one API version in the same library (as we do in Ruby), the tests for the multiple versions cannot coexist. The toplevel classes are defined in the generated tests for each API version, and thus conflict. In some cases, this won't make a difference because the generated classes are identical, and Ruby allows you to reopen classes. However, if there are any differences in these classes, the conflicts will cause problems. Currently, the credentials classes are causing test failures because each version has a separate credentials base class, so the toplevel class is trying to subclass multiple base classes, resulting in a compile error.
A reasonable workaround might be to mangle the generated class names to include the API version. e.g. MockVideoIntelligenceServiceCredentials_v1beta1.

I would just go with giving all the test helper classes versioned names. It keeps things straightforward.

Just for background:

Should these classes be in the libraries at all? (i.e. are they useful to library users, just artifacts of our testing, or something else)
Should some of these classes be in GAX instead?

They are specific to the tests and belong there. (e.g. mocks, etc.)

ping

@dazuma sorry for the delay on this one (@landrito and I will try to clear out the P1 backlog shortly).


arch: Add custom arch chip build support

Summary
Just like custom board build support, add custom arch chip build support.
Impact
Testing
Local test with custom chip build well.

I have no idea, it seems Linux arm-12 build is canceled for some reason.  I thinks it's not related to this PR. @btashton could you help to check it?

@xiaoxiang781216 LGTM too.  Just a note #1893 is now merged, I think if we want to extend arch context further, we need to use that specific target instead of the global one.

I have no idea, it seems Linux arm-12 build is canceled for some reason. I thinks it's not related to this PR. @btashton could you help to check it?

I've seen this happen to some PR recently.  I don't know the reason either.

I have no idea, it seems Linux arm-12 build is canceled for some reason.  I thinks it's not related to this PR. @btashton could you help to check it?

2020-10-19T03:09:00.8084547Z Found online and idle hosted runner in current repository's organization account that matches the required labels: 'ubuntu-latest'

My guess is GitHub had an issue because it assigned this VM but then timed out trying to start the job on it.

I have no idea, it seems Linux arm-12 build is canceled for some reason.  I thinks it's not related to this PR. @btashton could you help to check it?

2020-10-19T03:09:00.8084547Z Found online and idle hosted runner in current repository's organization account that matches the required labels: 'ubuntu-latest'

My guess is GitHub had an issue because it assigned this VM but then timed out trying to start the job on it.

Thanks, I get it.

@xiaoxiang781216 LGTM too. Just a note #1893 is now merged, I think if we want to extend arch context further, we need to use that specific target instead of the global one.

Yes, I agree with using arch specific target instead of the global one once extend arch context further.

I have no idea, it seems Linux arm-12 build is canceled for some reason. I thinks it's not related to this PR. @btashton could you help to check it?

I've seen this happen to some PR recently. I don't know the reason either.


Update console.redhat.com/preview for June 20, 2023

Remove "Free Tier" language from console.redhat.com/preview

Includes the following changes:

Adjusted use of mergeToBasename function to prevent additional forward slashes in routes
Add a "Not Found" page for non-existent instances on the instance details page
Replaced apiRequest with the useApi hook, addressing a bug where staging was directed to the prod backend
Split AWS marketplace link into NA and EMEA options
Remove "Free Tier" language from overview page


portable version request

hello, i am requesting a portable version for windows. i am not searching for something like the command --user-data-dir=
example: https://portapps.io/app/ungoogled-chromium-portable/
but official.

example: https://portapps.io/app/ungoogled-chromium-portable/

Does that even work? Doesn't it lose all settings / cookies / password when moving from one machine to another? From what I know Chromium has something that is dependent on some machine ID to protect those items...


Offcanvas Content - Responsive Design

Responsive design for the offcanvas content

:: use height for description instead of max-height


concurrency issues?

This is less of a 'fix something in mini_racer' and more of a 'am I doing this right?' issue.
There are two issues which are causing me problems:

data passed to ruby procs from js that is inconsistent

should be a string name, but is sub-array from another parameter

segfaults on that same line of code
https://gist.github.com/seanmakesgames/73aad7eaa078d04b7df421e8a34fe3b3

The issues happen inconsistently at least once a day, occurring more often with higher user concurrency.
Context:

ruby on rails 4 server
mongodb backend
delayed_job worker
mini_racer js engine

-> client sends request to server 'some_js_scriptname'
-> unicorn handles request on a worker process
-> worker process validates request then schedules job with run info
-> delayed_job worker daemon process picks up job (only one worker; 1 job at a time)
-> in delayed_job worker:
-> js source code is pulled from mongo
-> context1 is lazy-init (stays for entire life of worker process) to validate source code
-> context2 is created to run source code
-> context2 attaches procs for the source code to access db operations (above issue procs)
-> context2 evals
-> context2 stop
-> context2 result stored in mongo
-> worker picks up next job
-> client sends request to server: get_script_results
-> unicorn handles req
-> gets results for user from mongo
multiple users, multiple clients, multiple script calls, multiple result queries, one worker (atm) runs contexts synchronously.
I'd love to get a better understanding of where I might need to synchronize operations, or where things could be falling apart. Any help is appreciated.

Is this still happening on latest, cause my gvl detection code was way off...

I'm doing a production push tomorrow with a handful of concurrency fixes in ruby and latest mini_racer. Some of the ruby invalid state will go away. Should know if the other stuff is happening by end of day pacific time :)

looks like the ruby weirdness is fixed now (I was not handling FailedV8Conversion correctly in my procs)
The segfaults remain.
Here's the line of code they are happening on:
DatabaseConnection.client.database[collection_name.to_sym]
collection_name is a string from JS
the client is a Mongo ruby driver Client which is shared by all the calls in the worker process.

spoke too soon. The other thing is still happening as well. Just got an instance

One of my players did a code review of the c code (no experience with ruby c api) and had the following comments. (Breaking these up into separate comment blocks)
-----BEGIN COMMENT 1------
    result = rb_rescue((VALUE(*)(...))&protected_callback, (VALUE)(&callback_data),
                        (VALUE(*)(...))&rescue_callback, (VALUE)(&callback_data));

protected_callback has type
VALUE()(VALUE rdata)
and rescue_callback has type
VALUE()(VALUE rdata, VALUE exception)
Both are being cast to a function of type
VALUE(*)(...)
These casts cause undefined behavior. Pointers to functions of different types are completely incompatible with each other, and casting function pointers like that is undefined behavior. varargs functions in particular are extremely finicky beasts that have completely different calling conventions and semantics to normal functions. Note also that pointers to functions cannot be cast to void* (function pointers might be a different size than data pointers and void* is only required to be compatible with char* if memory serves)
There are more casts to varargs pointers at the bottom of the file in Init_mini_racer_extension.
-----END COMMENT 1-----
My quick googling into these comments yields:
http://stackoverflow.com/questions/11731159/in-c-is-it-ever-safe-to-cast-a-variadic-function-pointer-to-a-function-pointer
https://silverhammermba.github.io/emberb/c/#rescue
VALUE exception = rb_errinfo(); /* get last exception */
rb_set_errinfo(Qnil);           /* clear last exception */

----BEGIN COMMENT 2----
I saw a couple instances of this:
        isolate->SetData(0, (void*)false);
or
        isolate->SetData(0, (void*)true);
where that is later retrieved as:
        bool has_gvl = (bool)args.GetIsolate()->GetData(0);
This will PROBABLY work, but it also is technically undefined behavior. Although casting from a bool to a void* is specifically allowed (bools are integral types and any integral types are allowed to be cast to pointers), behavior is only defined when casting a pointer to an integral type of the same size or larger than the pointer. So, if the bool is 1 byte (it could also be 4) and the pointer is 4 bytes (it could also be 8), the cast from void* to bool is technically undefined behavior. I expect most compilers will do the right thing, but they could conceivably implement the cast from bool to pointer as setting a 1 bit in the HIGH order byte of the pointer, and the cast from pointer to bool by taking just the low byte (and thereby missing the bit they set). So, if this ever looks like it is doing the wrong thing, it could well be. There are zero guarantees that those will round trip.
----END COMMENT 2----

----BEGIN COMMENT 3----
THIS is also technically undefined behavior but almost certainly alright:
static void ruby_callback(const FunctionCallbackInfo<Value>& args) {
    bool has_gvl = (bool)args.GetIsolate()->GetData(0);

    if(has_gvl) {
        gvl_ruby_callback((void*)&args);
    } else {
        rb_thread_call_with_gvl(gvl_ruby_callback, (void*)(&args));
    }
}

the argument args is a reference, and then in the gvl_ruby_callback line, there is &args -- get the address of args. There is no guarantee that references have storage -- they could be handled as a pointer in a register or even completely optimized away -- so there is no guarantee that the address-of operator works on references. It almost certainly will, but again, it isn't guaranteed to as far as I can tell based on the spec. (even if it does have storage, I do not think, but can't find any help in the spec, that it is guaranteed that the &args is the same address as the object that was passed in as an argument. Thirdly, it is a const reference. I'm unsure exactly what is in a FunctionCallbackInfo, but if gvl_ruby_callback can modify it, this might be violating the method signature (the cast to void* suppresses the warning), so that might also cause issues.
----END COMMENT 3----

----BEGIN COMMENT 4----
I am a little concerned that it is POSSIBLE that the refs_count on IsolateInfo isn't initialized.
----END COMMENT 4----

ALL of this code works MOST of the time, so I don't think there's anything mostly wrong with any of it.
Unfortunately, my use-case puts mini_racer THROUGH THE RINGER. (100% usage for many-many hours every day; w/ some of the most arcane and awful JS you can imagine.)
Here are some of the exceptions in ruby, since I didn't post them before:
undefined method 'to_sym' for #<Array:0x000000071f92d8> (NoMethodError)
undefined method 'to_sym' for [2016-07-09 14:50:38 -0400, 1000, "rtd", "trust", "market.sell"]:Array (NoMethodError)
undefined method 'to_sym' for ["tier", 1]:Array (NoMethodError)
bundle/ruby/2.2.0/gems/activesupport-4.2.6/lib/active_support/core_ext/object/json.rb:34:in 'to_json': source sequence is illegal/malformed utf-8 (JSON::GeneratorError)
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer.rb:162:in 'eval_unsafe': String W�m���� �Ad@ is not valid UTF-8. (ArgumentError)
I'm calling #to_sym on collection_name which is a passed in parameter to my proc. All of the js being run in these cases is code that is run 1000s of times a day and has a low failure rate, but very uncommonly the same exception.
The segfault from the original comment is VERY consistent, but this is unsurprising because every proc that is called from JS hits this line of code.
Would love to hear your thoughts on all of this. (really appreciate your support!)

Probably unrelated, but the compilers and the clang static analyzer moan about this assignment: https://github.com/discourse/mini_racer/blob/master/ext/mini_racer_extension/mini_racer_extension.cc#L645

Assigned value is garbage or undefined

OMG that's totally relevant!
What happens if the for loop on line 635 doesn't happen or there's an issue converting params from js? We pass the uninit args to the ruby callback-
Or if there's a timeout exception while that loop is processing-- it can't finish the js half of the conversion?
I'm not sure, but this is definitely a spot!

length is properly checked on line 597
    if (data->length > 0) {
	result = rb_funcall2(data->callback, rb_intern("call"), data->length, data->args);
    } else {
	result = rb_funcall(data->callback, rb_intern("call"), 0);
    }

and data->args are not referenced, but I'm concerned about the mid-v8-conversion case (termination or otherwise). What is in the ALLOC_N data if it's not set by the for loop? (my assumption is QNil or something)
I looked at ALLOC_N and followed it all the way down to this func:
http://rxr.whitequark.org/mri/source/gc.c#4664
which does not seem to init the memory. Just seems to malloc it in the proper GC space & correct size.

Not sure this is super relevant, but found it when looking up ALLOC_N:
http://clalance.blogspot.com/2011/01/writing-ruby-extensions-in-c-part-12.html
re: mem leaks on exception.

Just a quick one, the weekend

Can you see what happens if you always convert to JSON and back prior to returning from evals (to IIFE wrapping and do a back and forth on the result)

Can you include some details about the location of the segfaults?

I would love to address every one of the issues raised, ask the reviewer if they can suggest fixes.

segfaults are happening at the following locations:
Segmentation fault at 0x00000000000030
Segmentation fault at 0x00000000000018

c-callstack:
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_vm_bugreport+0x51f) [0x7f94ede4813f] vm_dump.c:693
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_bug_context+0xcb) [0x7f94edcdb54b] error.c:425
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(sigsegv+0x3e) [0x7f94eddbbdae] signal.c:879
/lib/x86_64-linux-gnu/libc.so.6 [0x7f94ed8cdd40]
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(vm_search_method+0x4c) [0x7f94ede30b7c] vm_insnhelper.c:836
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(vm_exec_core+0x2625) [0x7f94ede38085] insns.def:1069
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(vm_exec+0x82) [0x7f94ede3b752] vm.c:1435
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(invoke_block_from_c+0x2f6) [0x7f94ede3ed96] vm.c:813
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(vm_invoke_proc+0xe0) [0x7f94ede3f060] vm.c:878
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_vm_invoke_proc+0x1a) [0x7f94ede3f11a] vm.c:897
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(proc_call+0x52) [0x7f94edcea652] proc.c:731
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(vm_call0_body.constprop.79+0x28f) [0x7f94ede3d08f] vm_eval.c:127
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_call0+0x1ba) [0x7f94ede3dcea] vm_eval.c:59
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so(_ZL18protected_callbackm+0x70) [0x7f94e68ea0b1] mini_racer_extension.cc:598
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_rescue2+0xc9) [0x7f94edce3709] eval.c:808
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so(_Z17gvl_ruby_callbackPv+0x36e) [0x7f94e68ea49f] mini_racer_extension.cc:649
.rvm/rubies/ruby-2.2.4/bin/../lib/libruby.so.2.2(rb_thread_call_with_gvl+0x1da) [0x7f94ede569ea] thread.c:1475
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so(_ZL13ruby_callbackRKN2v820FunctionCallbackInfoINS_5ValueEEE+0x85) [0x7f94e68ea733] mini_racer_extension.cc:676
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0x13f) [0x7f94e6dce01f]
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so [0x7f94e6959bcd]
bundle/ruby/2.2.0/bundler/gems/mini_racer-5f3641babe30/lib/mini_racer_extension.so [0x7f94e695a54f]

@SamSaffron The first bullet is much harder, as I have never seen this in my dev / test environment; only on production.
If I understand correctly you want something like:
var result = doWorkFunction();

(JSON.parse(JSON.stringify(result)))();

Reviewer is watching the thread, and will chime in soon, I'm sure. :D

Won't something like this work?
var __r = function(){...code to eval here...};

return JSON.parse(JSON.stringify(__r()));

Or perhaps
return JSON.parse(JSON.stringify( (function(){..code here..})() ));

also, can you ensure 100% that no context are being initialised prior to forking in unicorn. v8 is not fork safe.

wait, do you have a bit more depth on that backtrace I want to determine if it should be calling with or without the gvl

re: code -- do you want ALL of the code in the eval wrapped in an enclosure? (such that the procs are not in the same scope?
re: forking: all contexts are initialized inside of requests or in the delayed job worker process
for the requests, I'm confident they are all post-fork
for the worker, I'm reasonably confident that jobs are all run in the same long-lived process.
re: backtrace, you mean c-level backtrace that I provided before or a ruby one?

re:code, yeah just as a test
re:backtrace ... the c trace from the segfault is good, but does it have more frames/locations to it?

it has one more:
-- C level backtrace information -------------------------------------------
...previous scrubbed stack
[0x25511fb08ba7]
-- Other runtime information -----------------------------------------------

Just curious, what is the json-and-back-to-obj testing? Is it 'corrupt data before leaving js'?

I'm the one who reported these to @seanmakesgames.
So... first off, the comments @seanmakesgames called out as comment 2 and 3 are probably fine, they are pedantic, language-lawyer-y issues that I raised just on the off chance that misbehavior was traced to near those lines. I would be surprised if any sane compiler misbehaved in either case (then again, are any c compilers sane?), but they aren't strictly required to.
Comment 2 is relatively easy to fix, though: cast the void* to a pointer-sized integer type (like size_t) and then cast that to bool. (You could also go bool to size_t to void* on the input side for visual symmetry, but that isn't needed since bool to void* is specified). I'd be pretty amazed if any compilers don't do this implicitly, as I said. But that way you are doing 2 specified/defined casts rather than 1 undefined one.
Comment 3, I'm not sure how to fix that one, I'd need to really dig into the ruby internals. At a glance it looks like changing the method signature isn't necessarily straightforward since it is passed around as a func pointer in other places. I'm inlined to say, leave this one for now and just make a note of it on the off chance that some bug traces back to around this line. The only fixes I can think of are ugly and might invoke copy constructors (which then break reference semantics)....
Comment 4, likewise, I'd have to dig into the ruby to see how that is alloc'd. I just see a bunch of things like this:
ContextInfo* context_info;
Data_Get_Struct(self, ContextInfo, context_info);

[ this instance is from  line 456 ]

and ContextInfo includes an IsolateInfo which has the refs_count, and since I can't see where it was initialized, I flagged it to make sure. It might be easier, rather than trying to track all the places this happens, to just convert ContextInfo into a lightweight class with a default ctor that sets default values, so that no matter HOW they are made, that is set.
Which just leaves Comment 1, which is the one I think most likely to cause problems. In general, casting function pointers is just asking for trouble. The first, most obvious, option is to make those two functions have the proper signature that rb_rescue expects. It might mean you have to deal with varargs functions here, which is annoying and verbose, but it would get rid of the need to cast. There are other options, but since you can't change the ruby side of the code, most of them are nonstarters (i.e. you can't change the entire library to use functors because you'd need ruby to cooperate).
Also, related to those casts, on the same line I see (VALUE)(&callback_data), were callback_data is already a VALUE (so &callback_data is a VALUE*). I don't know what the VALUE type is, but casting a VALUE* to a VALUE looks suspicious to me. But, since this does seem to work at least some of the time, it can't be that bad.
As for the things discussed since.... I just am not familiar enough with how ruby and v8 work to comment on how to handle really poorly timed timeouts/etc that kill code in the worst possible places (other than, to quote @seanmakesgames, "don't do that" -- i.e. make sure that some code is protected from timeouts)

Thanks @dylanryan. these all look pretty straightforward. @SamSaffron I can provide a pr for some if it if you'd like.

sure any help would be more than welcome!
On Sun, Jul 31, 2016 at 9:33 AM, seanmakesgames<EMAIL_ADDRESS>wrote:

Thanks @dylanryan https://github.com/dylanryan. these all look pretty
straightforward. @SamSaffron https://github.com/SamSaffron I can
provide a pr for some if it if you'd like.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/discourse/mini_racer/issues/33#issuecomment-236396133,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAUXe1TVpfr7cIUXJaL9y39Uiw_qSqQks5qa99EgaJpZM4JWl7b
.

@SamSaffron putting together the PR now for the code review stuff, but I actually can't do the top-level to_json-and-back test. My sandbox relies too much on global variables. Can you tell me more about what you're trying to test with the top-level serialization?

In my googling for comment number 4, I'm getting pages saying that Data_Wrap_Struct does not automatically call '#initialize' in rubyland. Seems like this would have obvious symptoms?, but maybe not.
To do it, it looks like rb_obj_call_init needs to be called on the Data_Wrap_Struct object after the wrap call.
I don't know enough about the situation to make any kind of exec decision here, but there's a lot of conflicting examples online, the ones that omit the call to 'call_init' do not make mention of its existence.

After review of comment number 1, it looks to be that RUBY_METHOD_FUNC() is the appropriate way to pass functions into the ruby methods. However, it just resolves to a cast to (VALUE (*)(ANYARGS))(func) where ANYARGS resolves to ... in c++ and empty in c.
I did, though, find all examples that are passing in objects in the 2nd and fourth params, to be of Ruby-allocated VALUE type, whereas we are passing in c-structs and not Data_Wrap_Structs.
It looks like the arguments themselves are being properly marshalled into ruby VALUEs but the top level argument remains of struct typedef protected_callback_data
I'm not sure if this causes problems (lots of the struct members are properly typed as VALUE and marshalled), but it does seem wrong to cast memory that's not allocated with the ruby GC to a VALUE type.

Current overall status of comments:

Looks like it's fine, but switching over to RUBY_METHOD_FUNC() for the cast
implemented double-cast w/ reference comment
skipped. too difficult (v8 requires const & as param, ruby wants void*; copy constructor shenanigans ensue)
rb_obj_call_init might need to be called after Data_Wrap_Struct

https://github.com/discourse/mini_racer/compare/master...seanmakesgames:cpp_uninit?expand=1
outstanding questions:
a. Is there another way to get at the issue you're trying to test without top-level eval parse/stringify wrapping?
b. Does the protected_callback_data struct need to be Ruby-GC-alloc'd for passing into rb_rescue?
c. What happens if the ruby proc args allocation fails on line 635 during the loop? Do we get garbage for the rest of the arguments?

Empty args in c is at least sane, compared varargs in c++.
As best I can tell, in c, empty arguments simply means "A function exists, and it has this return value, and all the arguments are ints or doubles, and the number of args it has is whatever you tell me it is.". The compiler then looks at the callsite, and sees, ok, you are calling it with 3 arguments, so the function must have 3 arguments, and you are giving me a char and a short and a float, so that means int, int, double. Ok, cool, the function takes int,int,double. I'll plant all the usual code for that, and if the actual function is different, it'll explode at link time so not my problem." and then it proceeds as a completely normal function call using all the normal rules and calling conventions. (I am not sure how pointers are handled; I suspect that either void* is another type that is converted to, or pointers are converted to ints which will work on 32 bit and explode dramatically on 64 bit).
They key thing: The compiler knows exactly how many arguments and even what their type is in that kind of function call, whereas with varargs it has no idea (because it could be dynamic at runtime based on user input) and on most platforms just uses the stack and lets the called function deal with that. I'd still personally avoid those casts if at all possible (but probably not it seems), BUT assuming that the values are of convertible types, that is actually reasonably sane in c-land. C++? Not so much.
Varargs: "I have no idea but the called function will do it manually. If anyone is wrong, we'll get a runtime error. Well, best-case scenario, we'll get a runtime error, anyway. I really hope we'll get a runtime error.".
Empty args in c: "I know exactly what this is, it is whatever you tell me it is, and you wouldn't lie to me because you want your code to work, therefore this is correct. If it isn't, we'll get a link time error."
Of course, if those func pointers are casted BACK to the proper types in the C code on Ruby's side, then all is right with the world. Func pointers will round trip casts to any other func pointers safely.
Based on the code in that particular file and the extension... I'm guessing it is compiled as C++ despite being mostly straight C..... Well, if that is the officially sanctioned approach and it doesn't fail spectacularly on the majority of machines, maybe they got lucky and all i386/x86_64 systems just happen to generate code that just happens to work in this case.

@dylanryan it does look like they are not casting it before calling it. So I'm not sure.
http://rxr.whitequark.org/mri/source/eval.c#708
Either way, it does seem that this is 'as expected' for the ruby c-api :\ - so lets resolve other adjustments.

Bump.
Also here's another c-level exception. Might be caused by uninit mem?

Fatal error in v8::Isolate::Dispose()
Disposing the isolate that is entered by a thread.

bundle/ruby/2.2.0/gems/activesupport-4.2.6/lib/active_support/core_ext/time/calculations.rb:37: [BUG] Illegal instruction at 0x007f8cc50c99c9

I have confirmed that these segfaults are related to during script-run GC issues (most likely moving ruby/c-memory around when it's still in use by ruby or c++)
Please see #41


(Fix) Incorrect build sequence

The sequence of subtasks of build script is incorrect:
it copies everything to build folder and only then generates metadata for contracts. The reverse sequence should be. This PR fixes the issue.

@vbaranov
I found this

State off build folder

Fixed with the last commit

State of build folder

The rest look good to me,
Regards


Testing Istio mutual TLS authentication - cURL

The instruction on the page https://istio.io/docs/tasks/security/mutual-tls.html ask that you test mTLs using cURL. cURL is not installed on any of pods. You also cannot install it (unless I'm missing something). Maybe we need a different set of instructions or add cURL into the bookinfo app.

curl is present on the _debug images you get when you istioctl kube-inject --debug=true
we probably need to add that reminder now that the default changed

It's already done, see step 4 of this page.


Close to 100% test coverage

Closing in lieu of more clear tasks since we have contributors coming on now


Feat: Changing Tutorial sliding animation

Fixes #196.
Changing the start and end animtion of the tutorial so that it will start from bottom to top, then end from top to bottom.

Tested ACK 5d0bc3d. Thanks for the fix!


Issue after upgrading to v0.8.6

After upgrading from v0.8.5 to v0.8.6, litd will no longer start.
Here is the terminal output:
 ### --- things look normal up to here
2023-02-20 10:51:09.418 [INF] LITD: Starting LiT session server
2023-02-20 10:51:09.418 [INF] LITD: Starting LiT account service
2023-02-20 10:51:09.432 [INF] LITD: Starting LiT middleware manager
2023-02-20 10:51:09.433 [ERR] LNDC: Could not receive from interceptor stream: rpc error: code = Unknown desc = RPC middleware not enabled in config
2023-02-20 10:51:09.433 [ERR] LITD: Could not start subservers: context canceled
### ---- After this point, just normal shutdown logs

I've flipped the same config/auth back and forth between v0.8.5 and v0.8.6.  v0.8.5 continues to start normally and run perfectly. v0.8.6 doesn't start at all.
Is there some additional config or change in config that is needed for the v0.8.6 upgrade?
my lit.conf looks like this:
remote.lnd.rpcserver=<IP_ADDRESS>:10009
remote.lnd.macaroonpath=/auth/admin.macaroon
remote.lnd.tlscertpath=/auth/tls.cert

I'm starting litd with
./litd --configfile=./lit.conf --uipassword=hunter2

You need to add a new value the lnd configuration. Please take a look at the updated docs: https://github.com/lightninglabs/lightning-terminal/blob/master/doc/config-lnd-remote.md#quickstart (also see https://github.com/lightninglabs/lightning-terminal/issues/492).

I've also added a section to the release notes making this more explicit.


Update active-learning-suggestions.md

@DishaAgarwal : Thanks for your contribution! The author(s) have been notified to review your proposed change.


Error while trying to "interact" with intent classifier

dima@HAL9000:~/dev/DeepPavlov/deeppavlov$ python deep.py interact ./configs/intents/intents_dstc2.json
/home/dima/anaconda3/lib/python3.6/site-packages/h5py/init.py:36: FutureWarning: Conversion of the second argument of issubdtype from float to np.floating is deprecated. In future, it will be treated as np.float64 == np.dtype(float).type.
from ._conv import register_converters as _register_converters
Using TensorFlow backend.
[nltk_data] Downloading package punkt to /home/dima/nltk_data...
[nltk_data]   Package punkt is already up-to-date!
[nltk_data] Downloading package stopwords to /home/dima/nltk_data...
[nltk_data]   Package stopwords is already up-to-date!
2018-05-30 13:18:09.959 INFO in 'summarizer.preprocessing.cleaner'['textcleaner'] at line 37: 'pattern' package not found; tag filters are not available for English
/home/dima/anaconda3/lib/python3.6/site-packages/fuzzywuzzy/fuzz.py:35: UserWarning: Using slow pure-python SequenceMatcher. Install python-Levenshtein to remove this warning
warnings.warn('Using slow pure-python SequenceMatcher. Install python-Levenshtein to remove this warning')
2018-05-30 13:18:12.732 INFO in 'deeppavlov.models.embedders.fasttext_embedder'['fasttext_embedder'] at line 51: [loading embeddings from /home/dima/dev/DeepPavlov/download/embeddings/dstc2_fastText_model.bin]
2018-05-30 13:18:12.776 ERROR in 'deeppavlov.core.common.params'['params'] at line 112: Exception in <class 'deeppavlov.models.embedders.fasttext_embedder.FasttextEmbedder'>
Traceback (most recent call last):
File "fasttext/fasttext.pyx", line 152, in fasttext.fasttext.load_model
RuntimeError: vector::_M_default_append
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/dima/dev/DeepPavlov/deeppavlov/core/common/params.py", line 106, in from_params
component = cls(**dict(config_params, **kwargs))
File "/home/dima/dev/DeepPavlov/deeppavlov/models/embedders/fasttext_embedder.py", line 40, in init
self.model = self.load()
File "/home/dima/dev/DeepPavlov/deeppavlov/models/embedders/fasttext_embedder.py", line 53, in load
model = Fasttext.load_model(model_file)
File "fasttext/fasttext.pyx", line 154, in fasttext.fasttext.load_model
Exception: fastText: Cannot load /home/dima/dev/DeepPavlov/download/embeddings/dstc2_fastText_model.bin due to C++ extension failed to allocate the memory
Traceback (most recent call last):
File "fasttext/fasttext.pyx", line 152, in fasttext.fasttext.load_model
RuntimeError: vector::_M_default_append
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "deep.py", line 77, in
main()
File "deep.py", line 64, in main
interact_model(pipeline_config_path)
File "/home/dima/dev/DeepPavlov/deeppavlov/core/commands/infer.py", line 104, in interact_model
model = build_model_from_config(config)
File "/home/dima/dev/DeepPavlov/deeppavlov/core/commands/infer.py", line 43, in build_model_from_config
component = from_params(component_config, vocabs=[], mode=mode)
File "/home/dima/dev/DeepPavlov/deeppavlov/core/common/params.py", line 106, in from_params
component = cls(**dict(config_params, **kwargs))
File "/home/dima/dev/DeepPavlov/deeppavlov/models/embedders/fasttext_embedder.py", line 40, in init
self.model = self.load()
File "/home/dima/dev/DeepPavlov/deeppavlov/models/embedders/fasttext_embedder.py", line 53, in load
model = Fasttext.load_model(model_file)
File "fasttext/fasttext.pyx", line 154, in fasttext.fasttext.load_model
Exception: fastText: Cannot load /home/dima/dev/DeepPavlov/download/embeddings/dstc2_fastText_model.bin due to C++ extension failed to allocate the memory
32 Gb RAM on the machine

This problem can arise in two cases:

if memory is already allocated by another programs
if you use old fastText version. Current binary models are trained with fastText==0.8.22 version. Try to reinstall fastText form requirements, and if needed, re-download fastText models.


LPTIM Kconfig-related build failures for nucleo_g431rb

Describe the bug
Multiple tests fail to build for nucleo_g431rb with LPTIM Kconfig deprecation-related warnings:
tests/benchmarks/footprints/benchmark.kernel.footprints.pm
tests/kernel/profiling/profiling_api/kernel.common.profiling
tests/kernel/tickless/tickless_concept/kernel.tickless.concept
tests/kernel/timer/timer_api/kernel.timer.tickless
tests/subsys/pm/device_runtime_api/pm.device_runtime.api

To Reproduce
scripts/twister -v -N -M -p nucleo_g431rb -s tests/benchmarks/footprints/benchmark.kernel.footprints.pm

Expected behavior
Tests build without any deprecation warnings.
Impact
Use of deprecated features
Logs and console output
Loading Zephyr default modules (Zephyr base).

Environment (please complete the following information):

OS: Ubuntu 20.04
Toolchain: Zephyr SDK 0.15.2
Commit SHA: https://github.com/zephyrproject-rtos/zephyr/commit/432ff20a72e1bfbc95b40c589b55075d4d82f22e

Missing the Peripheral clock definition for the LPTIM in the stm32g4 DTS, This is to be now done with Device Tree
		lptim1: timers@40007c00 {
...
			clocks = <&rcc STM32_CLOCK_BUS_APB1 0x80000000>,
				 <&rcc STM32_SRC_LSE LPTIM1_SEL(3)>;
...

plus lse clock activation
&clk_lse {
	status = "okay";
};

Note that, on the stm32G431, the  LSE has a power consumption from 250 to 630 nA (depending on the drive capability)
to be compared with LSI  power consumption of 180nA Max


Added CPU blendshapes for GLES2

Basic software blendshapes for GLES2 renderer. So far, I have tested this on windows.
I'm not too experienced with pull requests so let me know if I should change anything
Fixes #36612
Bugsquad edit: This closes https://github.com/godotengine/godot/issues/48427.

The basic structure looks good. It is missing 2 things:

blend_shape_mode: can be "normalized" or "relative" (see here)
Blending of vertex attributes: this implementation only blends vertices, but blend shapes can contain any of the vertex attributes (see the blendshapes shader in GLES3)

It's difficult because clayjohn may be right about it only being used in GLES2. It's difficult to make that call at this stage, but it strikes me it would be a lot of extra work to change down the line, which would be unlikely to happen, so we end up having to commit to particular path early on. I can't immediately see any major downsides to doing it in the client code like the software skinning, compared to in the rasterizer itself.
Some thoughts:

Having a common reference software fallback covers us for all eventualities (e.g. driver bugs in either the blendshapes or the skinning in GLES3)
I'm slightly wary that the blendshapes and the skinning are, or could become intertwined (I haven't examined how they work at this stage, but e.g. tweening face is often used at the same time as skeletal animation). Attempting to have software skinning and hardware blendshapes (GLES3) could become problematic down the line. You may for example need to do the tweening before the skinning, which won't work with the CPU + GPU arrangement.
Shared code / techniques, we may be able to to share a lot of stuff for e.g. culling / logic between the two, which would be more difficult if it was split into software skinning client side and software blend shapes in the GLES2 rasterizer.

I'll try and have a proper look tomorrow as it is getting late here, and work out how the existing blendshapes work, I haven't looked or tried these (so maybe as clayjohn is more familiar he may be right with his intuitions).

Wonderful thoughts, as always. My responses below:

Having a common reference software fallback covers us for all eventualities (e.g. driver bugs in either the blendshapes or the skinning in GLES3)

The issue with putting the software implementation into client code is that users have to force enable software fallbacks if they think that any of their users will experience the driver bug. We need a solution that just works (TM).

I'm slightly wary that the blendshapes and the skinning are, or could become intertwined (I haven't examined how they work at this stage, but e.g. tweening face is often used at the same time as skeletal animation). Attempting to have software skinning and hardware blendshapes (GLES3) could become problematic down the line. You may for example need to do the tweening before the skinning, which won't work with the CPU + GPU arrangement.

With this PR, blendshapes and skinning have the same relationship that they do in GLES3. So if something is wrong when combing blendshapes and skinning then it needs to be addressed for both. If it does turn out to be an issue, then certainly we should kill two birds with one stone and implement software blendshapes client side.

Shared code / techniques, we may be able to to share a lot of stuff for e.g. culling / logic between the two, which would be more difficult if it was split into software skinning client side and software blend shapes in the GLES2 rasterizer.

This sounds very interesting, but I don't know enough about the software skinning to know if it will be valuable.
Overall, I think we should implement blend shapes in the rasterizer in GLES2 and then evaluate whether it makes sense to add a client-side software blend shapes implementation that replaces it (the same way that the current software skinning replaces the hardware skinning). I know it would feel redundant, but this PR won't be very much code even when finished) and it makes sense to me to have proper blend shape support in the rasterizer.

Ah I get you, sorry I got my wires crossed. You are meaning make a hardware implementation for GLES2 (I was for some reason thinking you meant a software CPU solution in the rasterizer), yes it didn't occur to me as I'm not familiar with blend shapes. So I'm in agreement that a hardware GPU version would be worth trying. :+1:

@clayjohn I have now added blending to normals, colors and uv maps and made it work on both normalized and relative modes. I wasn't sure how exactly weight/bones blend, but it seems to work anyways on my animated skeleton/blendshape test project.

Alright, I've added tangent and weight blending now.

Fantastic work to get this done in such a short time for a new contributor. I'm happy with if clayjohn has tested, it looks well done from a quick look.
Only question, as I'm not familiar with blend shapes in godot, can they be combined with skinning, and if so, are the results the same when using this with software skinning or using this with hardware skinning? The classic example would be using blendshapes for a face / head for talking, used in combination with skinning for the bones. But a simpler version would do to test it.

This results in slightly more expensive CPU-GPU transfers
Yes, i was also thinking about this situation especially on weaker platforms such as android. An idea I had was to only recalculate the blend shapes when the mesh has changed (blend values, blend data, mesh data, etc) with like a dirty flag or something. At least for now it would be fairly easy to find your bottleneck by simply hiding objects.

@clayjohn you said:

That being said, I'm not sure storing a GPU buffer per mesh, configured for each mesh's combination of compression formats is worthwhile.

I'm currently optimizing this implementation since performance was pretty bad in a real world case. I was able to implement a CPU/GPU buffer for each surface, and it works much better - framerate now only drops when the blend shape is changed. I justify the added memory usage by only creating it for meshes that actually has blendshapes. The previous code did upload shape data to the gpu without ever using it, anyways. I'm curious what your thoughts were regarding what you wrote, what should I look out for?

@parulina I was concerned about memory usage. If there is enough speed improvement then it is probably worthwhile.

Is it difficult to add this to gles3?

I'm currently optimizing this implementation since performance was pretty bad in a real world case. I was able to implement a CPU/GPU buffer for each surface, and it works much better - framerate now only drops when the blend shape is changed. I justify the added memory usage by only creating it for meshes that actually has blendshapes. The previous code did upload shape data to the gpu without ever using it, anyways.

@parulina Are you planning on making a PR with the changes?

I totally forgot! I can clean up/rebase and submit it for tomorrow.

That would be much appreciated. Thank you :)

Was CPU Blendshapes added to GLES3? I wasn't sure.

@fire No, this PR was specific to the GLES2 backend.