In query processing, it is my understanding that a substantial percentage of time is spent in hydration of returned rows from a result set. Some of the hydration process (SQLQuery.hydrateRecord()) could be improved slightly (e.g. pre-calculate some things, iterate over a pre-calculated array of "field descriptors" rather than iterating over the Map.entrySet() results...). Regardless of these small (but safe) improvements, the majority of the hydration time would still be needed.

The more costly part is the hydration itself. It seems to me that it is likely that the majority of hydration work that is done is not needed. In other words, for most buffer loads it is likely that only a small number of fields in a given row are actually accessed. The various buffer copy cases are exceptions but I think these will be a small percentage of the overall buffer loads in most use cases.

I think we should measure the following:

• how much query time is spent in hydration
• how often hydration is really needed

We've often discussed that it is likely that implementing the runtime parts of field list support (#2137) and calculating field lists where we can (#6721) will have some major performance benefit. Field lists would allow implementation of a partial hydration approach. I would expect that a field list would be rendered down to an array of "field descriptors" which would be used for the hydration loop as described above. By implementing partial hydration, we could get a large amount of the hydration time back for some number of statically calculable cases. This won't help us in the cases that cannot be calculated.

Lazy hydration is more of a runtime concept. The idea: only hydrate a given field when it is actually accessed. Doing this would require us to carefully map the lifecycle of the returned result set. Lazy hydration is possible so long as the result set still exists, but this won't be valid for cases where:

• the buffer scope is at an enclosing block from the block to which a query is associated (e.g. FOR EACH)
• the query is a "one and done" associated with something like a FIND, such that it doesn't outlive the FIND itself

It seems to me that the FOR EACH case (including the very costly multi-table cases) doesn't need to worry about the lifetime of the entire result set because only a single row can survive the scope of the FOR EACH block. Worst case, we could hydrate the last set of buffers as we exit the block and the rest of the result set can never be accessed again anyway. This seems like we could avoid a huge amount of hydration with this lazy approach.

Implementing laziness for FIND would require retaining the result set for the lifetime of the buffer.

Is it possible that the buffer could be scoped outside of the database transaction? If so, that would be another problem to resolve.

If we are to lazily hydrate records, any column data which was not loaded in the initial hydration pass will have to be loaded while our result set cursor is still "on" that record in the result set. A JDBC ResultSet is not a random-access construct; all movement among records is relative to the cursor's current position. The typical idiom of reading a set of results is:

ResultSet rs = <execute query>;

while (rs.next())
{
   // access column data in the result set for the next result record,
   // using an individual method call for each column from which we
   // want to read a datum
}

Once the cursor leaves the current record, it is not likely we can get back to that result row efficiently. If certain parameters are passed with the query execution to enable non-forward movement through the result set, one can move backward and forward relative to the current cursor position, but the backend needs to support this, and it will not be fast to get to some random record.

We will need to check whether the column we are reading has changed in the DMO and know whether the record has been deleted, so that we are not loading stale data from the result set.

If we are to lazily hydrate records, any column data which was not loaded in the initial hydration pass will have to be loaded while our result set cursor is still "on" that record in the result set.

As I noted above, more many cases like FOR EACH, isn't this already the normal lifetime of the record in the buffer? Unless I am misunderstanding things, there are some number of cases where we already match the lifetime of the buffer to the time during which we would have access to the result set. Such cases could easily access just the fields needed, when they are needed. Presumeably, this is a subset for most cases. Only some rare code cases and things like BUFFER-COPY/BUFFER-COMPARE would access all fields.

Once the cursor leaves the current record, it is not likely we can get back to that result row efficiently.

I'm not suggesting this. The idea is that for the cases where we must have access to a given record that is scoped longer than the result set, then we can copy that record only when we leave the scope of the result set. It should only affect a single record because only that one record referenced by the buffer can ever be accessed again. I'm thinking of a FOR EACH case as an example. Worst case, all records but one can be lazy.

Code posted in #5731-399 implements partial record hydration.

Greg, I think that #5731-399 doesn't implement lazy hydration. It just extends the usage of FWD Persistence API for external java applications in which "select" queries with incomplete list of fields are used.

Yes, that is what I calling partial record hydration.

Ovidiu, Constantin, Alexandru, can you please help me understand this snippet of code in SQLQuery.hydrateRecordImpl?

         pk = resultSet.getLong(rsOffset);
         Record cachedRec = session.getCached(recordClass, pk);
         if (cachedRec != null && !cachedRec.checkState(DmoState.STALE))
         {
            SIMPLE_QUERY_PROFILER.updateCacheHits(resultSet, 1);
            // we found an unSTALE CACHED record, do not bother reading from result set
            return cachedRec;
         }

         if (count == 1)
         {
            SIMPLE_QUERY_PROFILER.updateCacheHits(resultSet, 1);
            cachedRec = session.get(recordClass, pk);

            return cachedRec;
         }

In the first conditional block, we check the session cache for a DMO with the primary key retrieved from the result set, and we return it if it is not null and not marked STALE. However, only the null check is really useful, because if a STALE record was found by Session.getCached, it would have been evicted during that call, and null would have been returned. Session.getCached will not return a STALE DMO.

So, if we reach the second conditional block, we must have had a session cache miss. Then we check if count is 1, meaning we only have a primary key and no other data in the result set. At this point, even though we've had a cache miss, the first thing we do in that block is register a session cache hit, with a call to SIMPLE_QUERY_PROFILER.updateCacheHits. Then, we check the session cache again, using the same parameters as before, so we must get the same result back: null. I suppose the return of the null would be appropriate here, since there's nothing we can do to hydrate the record, if we only have the primary key. However, the JMX call seems wrong, and the second cache check seems unnecessary.

Unless I've misunderstood something (please let me know if you think so), I think the correct logic would be:

         pk = resultSet.getLong(rsOffset);
         Record cachedRec = session.getCached(recordClass, pk);
         if (cachedRec != null)
         {
            SIMPLE_QUERY_PROFILER.updateCacheHits(resultSet, 1);

            // we found a cached record, do not bother reading from result set
            return cachedRec;
         }
         else if (count == 1)
         {
            SIMPLE_QUERY_PROFILER.updateCacheMisses(resultSet, 1);

            // we found no record in the session cache for the given primary key and
            // we have no data with which to hydrate a new DMO, so return null; let
            // the caller work it out
            return null;
         }

Please let me know if you see any problems with this. Thanks.

Eric Faulhaber wrote:

In the first conditional block, we check the session cache for a DMO with the primary key retrieved from the result set, and we return it if it is not null and not marked STALE. However, only the null check is really useful, because if a STALE record was found by Session.getCached, it would have been evicted during that call, and null would have been returned. Session.getCached will not return a STALE DMO.

I am not sure about the STALE state of a DMO when doing a cache check. Session.getCached evicts STALE records before returning, but only if versioning exists (only available for non-temporary DMO). Therefore, is it safe to say that we cache STALE records only from the temp database?

So, if we reach the second conditional block, we must have had a session cache miss. Then we check if count is 1, meaning we only have a primary key and no other data in the result set. At this point, even though we've had a cache miss, the first thing we do in that block is register a session cache hit, with a call to SIMPLE_QUERY_PROFILER.updateCacheHits. Then, we check the session cache again, using the same parameters as before, so we must get the same result back: null. I suppose the return of the null would be appropriate here, since there's nothing we can do to hydrate the record, if we only have the primary key. However, the JMX call seems wrong, and the second cache check seems unnecessary.

The second call is a session.get, not the same session.getCached. This means that, we actually want a second SQL query to retrieve the rest of the DMO (to hydrate). Also, I don't think that the "caller" will work it out if we return null; I guess it will consider that the primary key doesn't belong to an existing record anymore. I can agree however that the JMX is wrong; it is a cache miss (I was mislead by the variable name cachedRec).

Thanks for the second set of eyes, Alexandru! I was staring at this too long and completely missed that the second call was get and not getCached.

I am not sure about the STALE state of a DMO when doing a cache check. Session.getCached evicts STALE records before returning, but only if versioning exists (only available for non-temporary DMO). Therefore, is it safe to say that we cache STALE records only from the temp database?

No, we only ever mark/detect a record is stale when versioning is enabled (as you noted, only for persistent DMOs). The javadoc for DmoState.STALE is misleading; staleness has to do with a context updating its version/instance of a DMO while that DMO simultaneously is being used in a different context (as a different instance representing the same record). Any change made in another session (which requires an exclusive lock) will make all other copies in other sessions stale. This is not possible for temp-table records, which are all private/local.

Constantin: in SQLQuery.hydrateRecordImpl (line 834 in trunk rev 14585), we are checking if the current result set row contains only a single field/column. If so, it is presumed to be the primary key, and we are either getting a cached DMO (if there is one in the cache with that PK), or allowing Session.get to instantiate and fully hydrate a DMO for that PK:

          if (count == 1)
          {
             SIMPLE_QUERY_PROFILER.updateCacheHits(resultSet, 1);
             cachedRec = session.get(recordClass, pk);

             return cachedRec;
          }

If count is 1, doesn't that mean we have a projection (i.e., primary key only) query? Something like (in FQL) this:

select foo.recid from FooImpl__ [where ...]

In that case, we should only be getting back the recid, not a fully hydrated record. Are we actually hitting this case in SQLQuery.hydrateRecordImpl for projection queries, or do they never get this far (i.e., the primary key is returned earlier and hydrateRecordImpl is never reached)? Maybe I'm misunderstanding what type of query gets us here?

For context, this specific case (i.e., to handle the single field case with Session.get) was added in rev 11347.1.137:

    revno: 11347.1.137
    author: Constantin Asofiei <ca@goldencode.com>
    committer: Ovidiu Maxiniuc <om@goldencode.com>
    branch nick: 4011a
    timestamp: Thu 2020-06-11 01:19:25 +0300
    message:
      Fixed SUBSELECT parsing and Fql2Sql usage.  Other misc fixes.
    modified:
      src/com/goldencode/p2j/persist/FieldReference.java
      src/com/goldencode/p2j/persist/QueryComponent.java
      src/com/goldencode/p2j/persist/orm/FqlToSqlConverter.java
      src/com/goldencode/p2j/persist/orm/SQLQuery.java
      src/com/goldencode/p2j/persist/orm/fql.g

It was a while ago, and went in with a huge update ultimately, but any recollections about the purpose of this change would be helpful.

Eric, I don't recall exactly which one, but this case of having a PK in hydrateImpl is originating from a kind of AbstractQuery - there was a case where this query was retrieving only the record PK. One of my status reports around that time mentioned this:

(fixed some issues, blocked with a ScrollableResults.get where the caller expects to get a full DMO).

See this javadoc in ScrollingResults:

   /** Scrollable result set;  primary keys or {@code Record}s */
   private final ScrollableResults<Object> results;

So there is cases when a query computes its results with only PKs instead of full records. I can track down a specific case, if is really needed.

OK, thank you. I've been reworking that method a lot and this didn't look right to me. I just wanted to know that there was a real use case behind it. I will retain it.

Eric,
Maybe a bit of history here helps. The code from SQLQuery.hydrateRecord() (now SQLQuery.hydrateRecordImpl()) evolved a bit over time at the same steps with RowStructure.

Initially, there was no RowStructure, just its dmoClass because that was enough for basic generated queries to instantiate and populate the fields of the record with data from the columns. The method was keeping track of the number of properties processed in case of row composed of multiple tables joined.

Then the RowStructure was added and it was keeping only the number of properties. There were two cases for automatically generated queries: either the full record was requested or only the PKs. In former case, the PK was, by convention, the first property in the result set and the method is able to fully rebuild/hydrate the original record. In the later case the data is evidently the PK and the algorithm goes as you noted above: first look for the record in cache then fetch it independently. Actually, SQLQuery.list(Session, List<RowStructure>) will do a more heuristic work. Analysing the number of expecting records in the row it will decide whether a row in the ResultSet represent a tuple of PKs or the expanded records. There are some javadocs with a bit of explanations.

Recently, the fields was added to RowStructure in order to support EXCEPT/FIELDS 4GL options. The FqlToSqlConverter will create the list of fields requested and populate the RowStructure accordingly. When the result is received, the hydrateRecord() will use this list for assigning only the restricted fields or use the full property map in its absence.

The current implementation relies heavily on initial conventions, primarily to minimize the data structure involved. So, if the count is one the value is PK. If you ant only one field, now the PK is mandatory (to identify the right recoed) so the count will be at least two. If needed, the RowStructure can be enhanced to provide additional information.

However, in case of hand-written SQL queries executed via the new persistence API the hydration is not possible if the caller does not pass the right RowStructure.

I've reread this issue and refreshed my mind. There are some key observations before moving on:

• Eric, 6720b exists (as you said), but didn't got the chance to check-out and review. Please pardon if my following points may contradict your current implementation.
• In #7045, the FOR-EACH queries are now parameters of forEach and forBlock. That means that the query is scoped to such block and FWD has full-control over its iteration. I think this helps a lot! We can basically control the scoping of every FOR-EACH query.
• This means that OPEN QUERY is the last issue here. However, I don't think we have a straight-forward solution for this - such queries can lose its records "in the wild". I am still to check Eric's solution, maybe there is a handling of such thing there.
• I found tons of examples where static inference of FIELDS clause would make lazy hydration less appealing. I thought we have a related task, but I may be wrong. This is about conversion time identifying the FIELDS clause for a query. I agree static approach is only sound, but no complete - however, we can buy some time here from the effective query time (less fields selected, serialized and returned).

I found tons of examples where static inference of FIELDS clause would make lazy hydration less appealing.

My original concept was that this was not needed. The idea was that we only hydrated when a field was actually accessed. That way we only ever do what we know we need to hydrate.

Eric's prototype (which he still intends to document) intentionally over-hydrates to match the existing design of FWD which assumes in many places that hydration has happened. I would hope that we would cut these back significantly to really minimize what has to be hydrated. Perhaps we can find different ways to solve the same problems or we can limit the extra hydration to a subset of cases that really need it. Anyway, I think that is the primary challenge of making this idea work.

This entry is intended to document the prototype implementation of lazy hydration in 6720b/14632-14634: what is implemented, what is planned, concerns... The branch needs a rebase to latest trunk, which I am working through currently.

Prototype Design¶

As discussed in previous notes, the idea is to fetch records from the database normally. However, instead of hydrating an entire DMO immediately before application logic uses it, we set up enough infrastructure to defer hydration of the properties of the DMO until they actually are accessed. The prototype is unfinished at this time and relies on some classes which are only partially implemented. It works only for the most basic test cases (simple FINDs and FOR EACH loops).

When Lazy Hydration is Applied¶

Only certain data access patterns support lazy hydration. Persistence APIs which gather and return a list of DMOs before using them cannot be lazily hydrated. Those which hydrate one record at a time, then use it with application logic, such as a single record retrieval or a scrollable result set, can support the concept of lazy hydration. We should strive to migrate cases which use a list pattern to use a scrolling pattern instead, where possible.

Currently, the database dialect determines whether lazy hydration is supported for a certain database type at all (see Dialect.useLazyHydration()).

If both the dialect and the access pattern supports lazy hydration, it is applied.

Implementation¶

Once a query is executed and a result set has been returned, and if lazy hydration is supported by the current dialect and the data access pattern, the JDBC result set is wrapped by an instance of LazyResultSet. This result set delegates every method call to the original result set. It also maintains an integer token which is incremented every time a method is invoked which would change the backing result set's state in such a way that would invalidate reading data from the current row. This includes moving the current row, closing the result set, etc.

The LazyResultSet is created by the following methods and is passed on, eventually, to SQLQuery.hydrateRecordImpl:

• ScrollableResults.get()
• ScrollableResults.get(DmoMeta, String, String[])
• SQLQuery.uniqueResult(Session, RowStructure)

In SQLQuery.hydrateRecordImpl, if a LazyResultSet has been passed in as the result set parameter, instead of fully hydrating the DMOs as we do currently, a Hydrator object is created by the LazyResultSet for each BaseRecord (i.e., DMO) created (or retrieved from the Session cache) for the row on which the result set currently is positioned. The Hydrator object receives and stores a copy of the LazyResultSet token, created at that moment. As long as the Hydrator token and the LazyResultSet token match, that Hydrator instance remains valid. Once the LazyResultSet token is incremented by repositioning, closing, etc. the backing result set, that Hydrator instance becomes invalid.

Hydrator stores a weak reference to the LazyResultSet. It is attached to the BaseRecord object, so that it later can be used to read data from the result set (assuming it is still valid at that time). When lazy hydration is active, the only datum that is read from the result set and stored in a DMO in SQLQuery.hydrateRecordImpl is the primary key of the current record. If the DMO already exists in the Session cache, it is retrieved from there instead.

All code paths to access a property must be re-routed to now go through the BaseRecord.getDatum method, where the actual, lazy hydration logic is implemented.

When application logic accesses (or sets) a DMO property and a Hydrator is present for that BaseRecord object, a bit set is consulted to determine whether that property is "live" (BaseRecord.liveProps). If so, the existing value of the property is returned from the BaseRecord.data array. If not, the Hydrator is tested for validity. If it is valid, the missing datum is read from the hydrator's LazyResultSet. It is stored in the data array, and liveProps is updated to set the corresponding bit. A similar operation must occur when setting a property (it must be read from the result set if not yet "live", so that the related ChangeSet object contains the correct baseline data).

In the event the hydrator is determined to be invalid (via a token mismatch with its LazyResultSet), it can no longer be used to retrieve a property from the backing result set and it is discarded (detached from the BaseRecord). In this case a LazyHydrationException is thrown. This is an unchecked exception. A handler for this exception is not currently implemented, but the idea was to catch this exception either in a common data retrieval method within the Record class, or perhaps further up the stack. The handler for this exception would force a re-fetch of the DMO's data by primary key from the database and perform a full hydration. However, the more I think about this use of an exception, the less I like it, both from a performance and an architecture standpoint. TODO: can we dispense with LazyHydrationException and handle the re-fetch within the orm package?

My initial plan was to forego further lazy hydration for this re-fetch and perform a full hydration in this scenario, since any additional round trip to the database must surely overshadow the performance benefit of lazy hydration, and we certainly would not want to make multiple such trips. Note that we need to ensure that use cases where a DMO snapshot or copy operation would cause such a re-fetch to occur do not represent common code paths. Currently, I think they do, as we take snapshots of DMOs at multiple places.

Unfinished Work¶

• Many LazyResultSet methods are not yet implemented. Those critical for initial testing of the prototype were implemented, but many methods are simply stubs. The good news is that these are quite simple, in that they merely delegate to the wrapped result set object, possibly incrementing the token if the state is changed for the current row.
• Record needs its data access methods reworked (only one or a few data types currently are handled) to use BaseRecord.getDatum instead of directly accessing the BaseRecord.data array. In fact, all logic which currently accesses the BaseRecord.data array directly needs to be refactored to go through BaseRecord.getDatum. Direct access to the data array will corrupt lazy hydration.
• LazyHydrationException needs to be handled or replaced with a cleaner/better/faster idea. The DMO re-fetch needs to be fully figured out and implemented.
• Logic which currently snapshots/copies all DMO data (which happens, for instance, as each record is unloaded from a buffer) needs to be re-evaluated and refactored to deal with this requirement differently. If not, we are still fully hydrating every DMO and taking even more of a performance hit because this is being done with an extra database round trip.
• More advanced testing, once the prototype is further along, to confirm that there is a measurable performance benefit from lazy hydration in real use, and with different database dialects.

Other Concerns¶

• We currently support to a large degree the 4GL FIELDS/EXCEPT options (implemented previously). This is in some ways similar to lazy hydration, but I want to keep these implementations separate. The underlying assumption of lazy hydration is that the 4GL application logic does not specify which fields are needed (or excluded) for a particular query, so we only hydrate those which are needed, in a just-in-time fashion. When a FIELDS/EXCEPT option is used, we assume that the 4GL developer explicitly intended the specified fields to be included/excluded. In fact, OE reports errors if a field is used that is not part of the developer's specification, it does not fetch those fields as lazy hydration would.
• How should DMOs already cached in the Session be handled in the context of lazy hydration? How should lazy hydration and session caching be handled generally? How useful is a partially hydrated DMO fetched from the cache sometime later (when the Hydrator almost certainly is invalid)? Is it defeating the purpose of caching if we very likely have to make another round trip to the database to re-fetch missing data?
• We need to ensure we are not introducing memory leaks with the additional Hydrator objects and any possible strong references to result set objects that should be cleaned up.
• My initial decisions regarding which dialects should support lazy hydration are over-simplified:
  • Persistent databases are assumed to benefit from it, though we have no confirmation of this, except for PostgreSQL (which test did not consider the cost of re-fetching).
  • It is assumed that H2 should not use (this form of) lazy hydration, because we only support in-memory use of H2 for production environments. However, this was based on the assumption that in-memory access would already be faster, but this was not confirmed with testing.

Task branch 6720b has been rebased to trunk rev 14762. New 6720b revision is 14766.

Alexandru, please review the code and the notes above and post any questions/comments you may have.

Eric, I prepared some questions here last week, but it seems I missed submitting them. I was kind of waiting for feedback without questions :/

When Lazy Hydration is Applied¶

This is my first concern so I will start with this. From the initial discussion, I thought this will only apply to very specific use-cases, rather than general cases. It seems that you rule out only scenarios where list is used, which makes sense. However, I was thinking of allowing this only for FOR EACH, FOR BLOCK, REPEAT kind of constructs, which have a very well defined scope. From your comment, I see you are planning to extend this to OPEN QUERY use-cases as well. This concerns me, as we can easily lose the DMOs in the wild with OPEN QUERY. For FOR-kind blocks, we will always do next until the last record. For OPEN QUERY, we can reposition at arbitrary times, so only the buffer used for the query will be hydrated properly at all times. All DMOs that were retrieved using the OPEN QUERY, but now reside in other buffers, will be invalidated, if they access unhydrated fields.

Your current approach is "bottom-up", in the sense that we will lazy hydrate almost everything and by performance tests will eventually reduce some cases (or maybe not). I was rather thinking of a "top-down" approach, cherry-picking some use-cases and eventually extending the solution to other constructs.

Implementation¶

Really good approach with the token versioning. I think this is light-weight as long as we carefully care for the DMOs and Result-Sets, not to cause memory leaks. I have no concerns here. Initially I thought of a listener approach where DMOs are eagerly invalidated if the lazy result-set moves, but this will require a listener list that may easily leak.

Unfinished Work¶

• I've seen performance decrease before when using getter instead of direct data access, but so be it - we can't handle lazy hydration without explicit getter calls.
• "Refetching" is the scariest word around this task. Without proper testing, it is hard to understand how often will these "lazy hydration" problems occur.

Other Concerns¶

• Session cache is a very delicate matter here. We risk using a stale information with high chances of refetching when we actually have the full-data, but unhydrated yet. This is something like (20% changes of going very fast as we don't refetch + 80% changes of going slow as we refetch vs 100% of going fast as we hydrate). Maybe we can decide based on the DMO (if it has >50% of its field hydrated, use it from cache, otherwise no).
• For FWD-H2 (in memory), we have some implementation that is quite good, but is slow due to the getter/setter things. Anyway, I would go ahead only with persistent dialects and let FWD-H2 in-memory to be implemented separately. When we have direct-access, we can even have a more special Hydrator.

With this being said, I am mostly good with the approach. The only concern is related to the use-cases we actually use this for and the session cache relevance.

I like this idea.

I applied lazy hydration only to tables where we know that the SQL number of columns matches the number of properties. I designed DmoMeta.sqlFieldCount for this exact purpose some time ago. Planning to use it now.

Does that also exclude extent fields when in expanded mode? It seems like this "offset" calculation can be done once at conversion time and then used always. In other words, can't we remember this offset and use it instead of a simple index?

Greg Shah wrote:

I applied lazy hydration only to tables where we know that the SQL number of columns matches the number of properties. I designed DmoMeta.sqlFieldCount for this exact purpose some time ago. Planning to use it now.

Does that also exclude extent fields when in expanded mode? It seems like this "offset" calculation can be done once at conversion time and then used always. In other words, can't we remember this offset and use it instead of a simple index?

I need to reanalyze the extent field. We can do some kind of mapping (FWD offset to SQL offset) to save us from ruling out potential lazy hydration candidates.

I've done some tests with 6720b/14768 on a customer application POC:

• the hot-spot of the investigation is how many times we need a second round trip to the database to load due to token mismatch: ~2.4% of cases we need to load the record again. All of these cases are successfully refreshed.
• there are no failed refreshes.
• indeed, some result sets are not lazily created due to the mismatch between FWD offset and SQL offset: ~16.8%. This is big enough to require fixing now, but small enough to be able to see some performance differences without fixing.
• note that there are less records than queries. This is because, I tracked only records that weren't cached / were cached but are lazily rehydrated. Only ~5% are not subject to lazy hydration / re-hydration, because the scrollable results was not lazy or the query was partial.

I will do a profiling test now, to have an intermediate status and set some expectancy.

• Added JMX to do some tracking for lazy hydration. Hopefully they aren't slowing the process too much.

I've done some profiling with 6720b/14769 and got exactly -2% improvement.

Optimization 1: Refresh only fields that weren't hydrated already
Optimization 2: Discard the hydrator if we hydrated everything due to successive calls to get/set

• This will improve the refreshing time, aiming only for the fields we didn't hydrate already
• This will decrease the number of useless rehydration.
• A negigable overhead is added in finding out if all fields are live.

Unfortunately, the changes didn't prove to be that drastic, improving insignificantly (-0.1%). This is mostly because of the fact that refreshes are happening very rarely on the persistent database + the unhydrated fields are usually >90% of the total number of fields. Bad lead :/

Avoid lazily hydrating from H2¶

I omitted the start-up because it was volatile (depending on the appserver start-up time).

The last statistic represents how many times we lazily hydrate, but excluding the times we "re-hydrated" cached items (replacing the old hydrator with a new one)

Some conclusions:

• There are way less lazy hydration attempts on persistent database (~4.1k lazily hydrated fields)
• Suprisingly, there are still >60% records lazily hydrated / rehydrated. This means that each hydrator is used to hydrate 0.5 fields on average (comparing to the previous 3 fields on avg). This is something that can be used in our further solution research.
• As expected, there are <30% lazy result-sets. Most of the result-sets that were lazy before were on the H2 database.
• Again suprisingly, 3% of the refresh attempts were for the persistent database. All other ~1k loads were for H2 database.
• The new statistic (RECORD LAZILY HYDRATED WITHOUT REHYDRATE) shows that only 12% of the hydrators are for new DMO, ther other 88% are used to rehydrate (replace an old hydrator).

I will attempt a profiling round with lazy hydration only for persistent database. I am curious if the whole process of managing lazy hydration on the H2 database was an overhead all along. I also want to stress out that only 8% of the lazy field access attempts were on the persistent database.

Unique result¶

Further, I have some concepts of supporting lazy-results for uniqueResult:

Scope result-sets to DMO lifecycle¶

• Attach the result-set to the hydrator just like we do now
• When the DMO is evicted from the session cache, it will also close its underlying result-set

This may be dangerous for UNDO tables which can bypass the dmo cache limit in Session. This will guarantee that no refresh will happen. However, this may be a performance concern as we need to reparse statements of result-sets that were not closed yet.

Queue the eviction of result-sets¶

• Don't close the result-set and let it generate hydrators when requested.
• Add these result-sets in a queue (size 1024 for a start)
• Discard (i.e. close) the result-sets when evicted from the queue.
• The same lifecycle is used for the statements.

This way, we keep the result-sets opened to support lazy hydration, but evict them periodically to avoid memory leaks. At worse, we end up with 1024 result-sets opened, but their SQL statements are still unusable.

Cache result-sets¶

• This is similar to the previous attempt, but use a LRUCache
• The key is the SQL used when computing the statement
• Each time we run uniqueResult, we check if we can close an in-use statement (so that we reuse it). This will close the underlying result-set to close and force refreshing

This is an optimized way of the previous idea, but we add the LRUCache overhead. Further, we either limit the size of this cache or let it expand to match the lifecycle of the DMO

Approach¶

If you have any feedback, it is much appreciated. I will attempt some of the ideas above next.

Avoid lazily hydrating from H2¶

Tested and had only -0.8% time improvement comparing to baseline. This means that the _temp database benefits from lazy hydration (curiously, even more than the persistent database). This makes sense from the POV of the statistics in #6720-32, but are quite odd as _temp is not quite slow due to hydration (de-serializing data from an external server). Anyway, the only fact that we don't have to iterate the full _temp record is now a performance bonus.

Conclusion here: we should also keep lazy hydration for H2.

Unique result¶

The first solution of scoping the lifetime of the result-set to the DMO is not quite logical: the session should have notified the dmo, that notified the hydrator that notified the lazy result set etc. This whole chain is quite reversed comparing to the dependency logic we have. Thus, I dropped Scope result-sets to DMO lifecycle.

New analysis after implementing Queue the eviction of result-sets is below:

• there are 8.6k new lazy result-sets; 2.3k others are not lazy due to field order mismatch (to be handled)
• the number of valid tokens increased with ~4k and invalids with 80. This is reasonable increase. I expected way more valid tokens to pop-out, but I think less is better, right? (less fields being hydrated)
• the number of records lazily hydrated increased with 1k and fully hydrated decreased with 100 (most probably from the previous optimizations). This is not promising :/ I suspect that out of the 8k new unique queries, only 1k end up being hydrated and not found in the cache or found in the cache but were rehydrated.

I attempted some profile, but unfortunately 2048 cache size determined tons of "reparsing warnings". All of these signaled a downgrade of almost +5% from baseline. I am planning to rework it to actually use the Cache result-sets variant where we ensure that we don't reparse statements. Before doing that, I want to recheck this approach with a very small queue (size 16 eventually).

Fix datetime-tz and extent cases¶

Next on my list is to fix the the datetimze-tz and extent fields that may cause offset mismatches. AFAIK, expanded extent fields are not a problem, as the DMO is actually considering each expanded field as a different index. However, I need to do some more testing in this area. Anyway, by fixing this, we can push almost 20% queries into lazy hydration (for my test cases).

Alexandru Lungu wrote:

Fix datetime-tz and extent cases
Next on my list is to fix the the datetimze-tz and extent fields that may cause offset mismatches. AFAIK, expanded extent fields are not a problem, as the DMO is actually considering each expanded field as a different index. However, I need to do some more testing in this area. Anyway, by fixing this, we can push almost 20% queries into lazy hydration (for my test cases).

I am not sure I understand why dtz are a problem for (partial|lazy) hydration. The name/type is available for each fetched column even before the request (select) being sent to DBMS and it should be also available in RowStructure companion object.

Talking about RowStructure. I think we can improve performance here by removing the fields Map<String, Property> and replacing it with a simpler and faster BitSet, if we impose that columns in a request are ordered by their property id. As you might already guessed by now, the BitSet would contain the indices of the requested properties. The properties will be extracted directly from the RecordMeta.getPropertyMeta(false) array.

I am not sure I understand why dtz are a problem for (partial|lazy) hydration. The name/type is available for each fetched column even before the request (select) being sent to DBMS and it should be also available in RowStructure companion object.

Well, it is not a big problem. It is just that it wasn't trivial to have it set for these kind of columns, so I skipped the tables using such fields just to reach some performance analysis / analytics asap. TL;DR, the datetime-tz fields have two SQL fields in the underlying result-set. Thus, all properties after a datetime-tz property (made out of 2 columns) will have their offset not in line with the FWD property order (e.g. char field no. 15 is on the 16th column in the result-set, because 5th and 6th columns were representing the same datetime-tz property).

Mind that in RowStructure we have an empty fields if we want all fields (which is the common case in fact).

Talking about RowStructure. I think we can improve performance here by removing the fields Map<String, Property> and replacing it with a simpler and faster BitSet, if we impose that columns in a request are ordered by their property id. As you might already guessed by now, the BitSet would contain the indices of the requested properties. The properties will be extracted directly from the RecordMeta.getPropertyMeta(false) array.

You are right, but this brings to my second point. If we partially hydrate (request only some fields from the DB), then I don't have a solution yet to integrate it with lazy hydration (extract the data from the result-set later on). Again, this is not a big problem - it was my intention to rule it out so that I can get some results asap.

For both problems above, we need a mapping between the FWD property position in data and SQL column position in the DB tables.

The hydration works now by iterating the columns of the ResultSet. The DataHandler s from com.goldencode.p2j.persist.orm.types automatically advance the necessary number of columns (for the moment all 1 with the exception of DatetimetzType) and return the number (methods readProperty() and setParameter()) of places processed. There is propertySize() property which can return the same value for you, if the previous ones are not involved.

Ovidiu Maxiniuc wrote:

The hydration works now by iterating the columns of the ResultSet. The DataHandler s from com.goldencode.p2j.persist.orm.types automatically advance the necessary number of columns (for the moment all 1 with the exception of DatetimetzType) and return the number (methods readProperty() and setParameter()) of places processed. There is propertySize() property which can return the same value for you, if the previous ones are not involved.

Indeed! The "difficulty" here was that we can't compute the proper offset on random access withuot actually iterating all properties from before. Thus, we need to do the mapping at DmoMeta generation. This can be a simple array (i-th position is the array holds the proper position in a ResultSet that selects all properties in order - the very usual case). For partial hydration, we can't compute it in advance, so we need to compute the mapping for each LazyResultSet / Hydrator.

Ovidiu, I have the idea pretty clear in my head; thank you for the guidance. I delayed this approach just because it represented only 20% of the cases in my tests, so I wanted to have some preliminary results on the other 80%. Currently, the improvement is -2%. If we are to tackle the rest of the 20%, maybe we can reach -2.5%. Of course, I will get them implemented sooner rather than later. My initial hopes were rather close to -5% :), so that is why I am "squeezing" the current 80% of the tests. Note to myself: Pareto was right, work really revolves around the 80/20 ratio :)

Unique result¶

I improved the solution for unique result. It uses a LRUCache per session to "delay" the closing of statements and result-sets. This way, it gives the lazy hydration a chance. Results are quite good, improving the performance to -2.5% comparing to the baseline:

• There is a LRUCache (of size 1024 now, but it seems way too big) in session. The key is the SQL to be run and the value is the last result-set obtained on this session by running that SQL.
• Each uniqueResult will attempt to remove and close the cached result-set for the statement it is going to run (if any).
• It will run the statement and if the result-set will be lazy, it will register it to the session result-set cache.
• Of course, eviction policy, clearing and closing of the session triggers result-set closing.

ProgressiveResults¶

This is a new topic of discussion here. I've seen Eric disregarded .list for bucket 0 in ProgressiveResults to honor lazy hydration. However, this first batch is very used. Due to these changes, there are lots (~200) messages from c3p0 signaling it has to reparse some statements. This makes sense as .list was closing the statement immediately and now with .scroll, the statement is closed when the FWD query ends (or the delegate is changed).

I readded .list to see the performance difference. With this added back, c3p0 is running fine (without the need to reparse statements).

• There are 6.5k less records (re-)hydrated due to the fact that there are 5.5k less scrollable queries made lazily. This makes sense, as these are now .list operations. These implies less valid tokens, etc.
• From observation, there are less statements reparsed

The performance I get by reintroducting list for ProgressiveResults is way worse (+3.5%). However, I am concerned by the fact that having list for ProgressiveResults and lazy hydration for other items is that bad comparing to the baseline. It means that the lazy hydration is an overhead for other items after all (I just need to detect where the lazy hydration under-performs). Are that 1.1k refreshes that bad? Or maybe my checks regarding datetime-tz fields?

Before moving on, I want to detect if we are falling behind with all these changes. I will recheck the changes first thing tomorrow morning and retest - hopefully there is no testing mistake I've done here.

Alexandru Lungu wrote:

Indeed! The "difficulty" here was that we can't compute the proper offset on random access withuot actually iterating all properties from before. Thus, we need to do the mapping at DmoMeta generation. This can be a simple array (i-th position is the array holds the proper position in a ResultSet that selects all properties in order - the very usual case). For partial hydration, we can't compute it in advance, so we need to compute the mapping for each LazyResultSet / Hydrator.

If we switch to BitSet representation and the columns are in a fixed given order the we need to iterate the BitSet, not the RecordSet. You can find the offset of the property k in current row by the formula:

i_rs(k) = SUM( property(i).getType().propertySize(), for all i < k )

If incremental processing of the properties in BitSet is not possible, the i_rs(k) can be computed at first access and cached as an index array (i_rs[k] = @i_rs(k).

I run some regression testing and they passed. I also got them a second round to extract some insights.

• The same "good" ratio between valid/invalid tokens is kept here.
• There are way more lazily hydrated records and lazy result sets (over scrollable and unique) in this test-case.
• Still no failed refresh.

With the latest changes (including lazy hydration for records with datetime-tz fields and normalized extents), the performance improve is only -1.5% now. My previous attempt was -2.5% (that excluded datetime-tz and norm. extent fields).
Even if the statistics are better, I am still struggling "descending on the performance ladder". I've run the LTW AOP tests we have and I couldn't spot anything out of the ordinary from Hydrator or BaseRecord.

Committed 6720b/rev. 14471

Status update:

• RowStructure: interface which allows defining a row structure (how the fields are selected from the database for a single DMO)
• AbstractRowStructure: base implementation. This stores only some base information as the DMO class.
• FullRowStructure: row structure that is used when not using any FIELDS clause (we want all properties). This will be used in most of the cases where we don't have partial hydration. We can even cache this instance (one per DMO).
• OrderedRowStructure: row structure that is implemented using a BitSet. This is right according to your suggestion of avoiding maps and keeping a bitset for the selected fields, presuming the fields are in order.
• UnorderedRowStructure: row structure that is implemented using LinkedHashMap. This is what we had now and keeps the columns of a DMO unordered.
• AdaptiveRowStructure: row structure that starts as an OrderedRowStructure, but if the order is broken, it will invalidate towards an UnorderedRowStructure.

This is an optimistic approach, hoping for a good row order. I am doing this to avoid any unexpected direct requests to our persistence layer that doesn't honor the fields order (from out-side FWD maybe). The only down-side here is that the AdaptiveRowStructure may invalidate and the properties are reiterated.

I just finished setting this up, I am now reworking SQLQuery to honor this new suite of row-structures. The goal here is to make lazy-hydration work only with FullRowStructure and OrderedRowStructure in order to have a proper identification of fields. In case of FullRowStructure, we keep the current implementation. In case of OrderedRowStructure, we do the lazy offset mapping.

That is an interesting approach. I did not realized that level of specialization would be necessary. This class was a simple int parameter in the initial implementation :-). I assume these classes have specific methods used for interpreting the result (including the hydration). There were some discussions related to calling related APIs from hand-written code, in which the result-set is not mandatory a projection (PK list) or (partial) records. In this case the Java data types should (probably) be kept and not mandatory wrapped as BDTs. Having various RowStructure allows the caller to specify this, too, maybe even with anonymous classes/ lambdas.

Is there are reason for having a RowStructure interface and an AbstractRowStructure abstract class? If not maybe we can collapse them to a flatter inheritance tree.

Looking forward to see the new code!

Ovidiu, I finished the implementation completely and tested it over a large POC. However, I created a separate branch I intend to merge independently from lazy hydration. The changes were hard to test with lazy hydration on anyway.
Thus, please review 6720c - I didn't profiled them yet, but they are mostly refactoring, so I don't except a large performance change - but a way to allow us couple lazy hydration with partial hydration.

My goal before merging 6720c is to ensure the overly used FullRowStructure is as fast as possible. I am already doing lots of presumptions comparing to the previous implementation to allow us to read the properties without any overhead.

Mind that all structures were tested on POC - only 3 invalidations happened in the process, but they were very slim (after 2 properties).

I grabbed 6720c and looked at the latest changes. Indeed, I do not see a reason for a change in performance. The hydration (and some related operations) was moved to new classes, but it's mainly the same.
However, I am not sure I fully understand the new mini-architecture. I need to read the new code one more time for that. I will write the detailed review after that.

Ovidiu Maxiniuc wrote:

I grabbed 6720c and looked at the latest changes. Indeed, I do not see a reason for a change in performance. The hydration (and some related operations) was moved to new classes, but it's mainly the same.
However, I am not sure I fully understand the new mini-architecture. I need to read the new code one more time for that. I will write the detailed review after that.

• In case of a full row structure, we can access the result-set mostly on the same indexes as the request. So, we can arbitrarily hydrate one property without much index mapping fuss.
• For incomplete records, so in case of an ordered / unordered row structure, we can compute the index mapping (and eventually cache it as you suggested). Thus, the partial hydration will do some kind of index mapping:
  • In ordered row structure, we keep a bitset. Checking if the property was retrieved from database is very fast.
  • In unordered row structure, it is quite hard to detect if a property was retrieved from the database or not - I will need to add a bitset to the unordered structure as well to manage that.

The whole overhead here is to be able to hydrate only one random property, but with different methods (faster in full row structure, slower in ordered row structure and even slower in unordered row structure). Maybe we can even rule out partial hydration in some cases (if the DMO is incomplete and the number of columns requested is less than a threshold?).

Alexandru, sorry for the delay. The size and the nature of changes (class tree, virtual methods) required ore time than I initially expected.

First, a question: how slow is the hydration of a field? I did not do a profiling here (I might do it in the future), but you may have the answer. For example, hydrating only 50% of the fields of average sized record (let's say 10-20 fields). This is a difficult question: the integer, logical and may character data are fast, but date, decimals and the rest, may be visible on the timer.
I am asking because I see that the code for supporting lazy hydration (not partial hydration - when some fields of a record are never fetched from database; this is handled by a different task by detection of the filed set at conversion time) is getting more and more complex. I hope this does not weight more than the full hydration if a record is hydrated in two or more 'stages'.

Now the review:

Good job. A lot of new code, which should particularize each case and optimize hydration independently. The code seems logical now, after re-reading it several times. I thought at first to spot some issues with reserved properties (because they have negative property ids), but now I think they are correctly hydrated.

Generally, I think passing the DmoMeta object instead of the DMO interface as parameter, is better, if possible. This is because the table metadata is stored there, including the DMO itself, but when the meta data is needed, obtaining it knowing the DMO implies a lookup over a large map (note to self: investigate the registries of DmoMetadataManager from the viewpoint of (initial) dimension, load factor, in cases of large enough client applications).
We agreed to avoid normalized/denormalized terms. Instead we should use not expanded/expanded.

My notes below are mainly related to code formatting/style and small local optimizations than actual bugs:

• AdaptiveRowStructure.java:
  • the javadocs of methods addRecid(), addMultiplex(), addProperty() are missing the @return tag;
  • hydrate() method is missing the @throw tag;
  • extra parameter count in c'tor's javadoc;
  • the addProperty() parameter name in javadoc does not match the argument name;
  • in invalidate() method: instead of comparing the prop.name (strings), the property.id should be tested with ReservedProperty.ID_PRIMARY_KEY and ReservedProperty.ID_MULTIPLEX respectively. They are integers so a switch is optimal;
• DmoMeta.java, FqlToSqlConverter.java, ScrollableResults.java, DirectAccessHelper.java: missing history header entries
• RowStructure.java
  • imports, preferred * instead of individual classes. Are there name collisions I failed to see?
  • as for AdaptiveRowStructure, the javadocs lack the @return and @throw tags;
• SQLQuery.java:
  • missing history header entry;
  • line 798/799: if the rowStructure had DmoMeta as member, the dmoInfo and recordClass would be obtained directly (see previous paragraph in this note)
• AbstractRowStructure.java
  • * imports instead of individual classes;
  • I see you added DmoMeta as a member here, by lookup from DmoMetadataManager, but the best solution is to have this the other way around;
  • addProperty() method is missing the @return tag;
  • hydrateExtents() lacks and @throw tag;
• FullRowStructure.java:
  • see above (imports, javadoc tags);
  • in hydrate(), when skipping the PK and _multiplex I would test whether the next() property is the right one. This would add a bit of robustness to code;
  • isIncomplete(): missing the @Override annotation
• RowStructure.java: see above (imports, javadoc tags)
• OrderedRowStructure.java
  • see above (imports, javadoc tags)
  • addRecid(), addMultiplex(), adding the PK or _multiplex twice (or more) will return true and continue incrementing the count.
  • getProps(): RECID_PHASE and MULTIPLEX_PHASE can be also static. Maybe use ReservedProperty.ID_PRIMARY_KEY and ReservedProperty.ID_MULTIPLEX?
    • in hasNext(), the phase advances. I think that's incorrect. Calling hasNext() multiple times while in RECID_PHASE and MULTIPLEX_PHASE will advance the iterator;
• UnorderedRowStructure.java
  • see above (imports, javadoc tags);
  • hydrate():
    • line 241: fieldCnt cannot be 0. If it was (fieldCnt = fields.size()), then the execution would not have entered the for loop. What was the desired meaning of this code?

Committed 6720c/14847 including the suggestions you made.

• Regarding fieldCnt, it should have been 1 to short circuit projection queries. Internally, all our projection queries are OrderedRowStructure, but anyway, I've honored this for UnorderedRowStructure as well. That change is irrelevant in our current state of affairs.
• Used ID_MULTIPLEX and ID_PRIMARY_KEY instead of the phase variables. The iterator works properly. If you call hasNext, the phase will advance only if it points to an invalid state (phase is recid but there is no recid). Otherwise, the method is idempotent.
• Renamed all denromalized/normalized references into not expanded/expanded.

I can't give you an answer on the hydration timing on the latest changes. On the initial code, there was clearly an improvement of ~2%, but was omitting the partial hydration (see #6720-41). I wanted to go all the way and support lazy hydration for partial structures as well. But this wasn't quite possible without this refactoring - allowing me to do the proper mappings between the requested fields and their result-set positions. Is it worth it? I don't know; maybe the mappings will slow down the process such that doing the lazy hydration will be in fact slower. At that point we can certainly rule out partial hydrations from lazy hydrations.

Anyway, I think the new architecture is worth having anyway, as we can optimize it "per-case", have a distinction between partial hydration / full hydration and "custom hydration" (outside FWD).

This was largely tested with a customer app and its POC (regression + performance). I intend to have it to trunk, rebase 6720b, and resume work on lazy hydration. Let me know what you think,

I am glad you already tested the implementation and you found no regressions. I re-analysed the iterator from OrderedRowStructure and, indeed, it looks correct. I was fooled by the cascading if s which makes the 'cursor' jump directly to right phase. Nevertheless, it is a bit unusual to change the internal state of the iterator in this method.

• are getDmoMeta() and getDmoClass() methods necessary? I understand that the values they return are used only 'internally' within the classes extending the RowStructure. I think a better approach would be to make those fields protected and final and access them directly (the dmoClass already is). I do not expect the result to be visible in profiler, but it might help;
• AbstractRowStructure.addProperty() still lacks the @return javadoc tag;
• AbstractRowStructure.toString() I would replace the RowStructure literal with getClass().getName() for easier debugging;
• FullRowStructure, line 211, the comment should read "skip _multiplex".

Fixed the last concerns. Committed 6720c / rev. 14848.

Redoing quick profiling and regression tests now for 6720c and preparing for merge. I am planning to resume the work on 6720b asap.

Completely fixed 6720c. I had some "invisible" regressions there. Also, it was quite confusing why the AdaptiveRowStructure was invalidated so often. I misused the RecordMeta API - there are close to no invalidations now.

Performance-wise, it is still on par with the baseline.

6720c is ready for merge.

You can merge to trunk now.

Branch 6720c was merged to trunk rev 14869 and archived.

Resuming work on 6720b

How far along is 6720b?

I am "battling" with a small regression when averaging the new row-structure architecture with lazy hydration. I was also ready to merge only lazy hydration of full records to have "something" on this merged, but even there we have a problem. I am roll-backing some changes to see where the problem occurred, but is a bit tedious. I will do my best to get a working solution by the end of this week.

Eduard, please assist the regression hunting here. I am aware that you have a more suitable environment for regression testing and debugging, so feel free to check-out 6720b, move it to 7156b and start regression testing. I can do the rebase for you as I am more familiar with the details if needed. We can have a talk on the changes here. Thank you.

Is this far enough along that we can measure performance, or are the regressions blocking testing? We need to understand the impact for both PostgreSQL and MariaDB.

On the POC, it regresses and crashes. I didn't attempt the tests on the new test suite.

Short update from a live discussion with Eduard: the regression seems to be related with the BaseRecord.getData calls that expose the "partial" data array out-side the record. This is not OK, because the data consumers are not aware of the lazy hydration encapsulated in BaseRecord. From my understanding, Eduard found this issue in a snapshot attempt of the record. The data was copied from a cached record to a snapshot record.

There are many places where getData is used, but I think that some of them don't actually require the fully hydrated data array or some of them can share the same hydrator. The trivial fix is to fully hydrate when doing a getData, but won't this nullify a great amount of effort invested in lazy hydration?

Eduard is working on analyzing how often we will fall in the pitfall of fully hydraation due to getData calls.

On delete of a temporary buffer and a deleted buffer is placed in before table.

But to make this copy a snapshot of the current buffer is made, and this snapshot is copied into before buffer.
However, to create a snapshot copy method from BaseRecord is called, where data array is copied without copying hydrator and liveporps.

And if the current record has some fields that are not hydrated, and unknown value for this fields is propagated into before table.

Lets go ahead with some statistics. From where is getData called and how many times. Please find the back traces of getData, add JMX counters and report how many times and from where it is called (in 7156b without 6720a). To test this, you can add getData() as in 6720a.

I changed BaseRecord.copy, to set hydrator and liveProps of the new record

public void copy(BaseRecord from)
   {
      // TODO: BLOB data is mutable, needs to be duplicated explicitly
      int len = this.getLength();
      this.id = from.id;
      copyArray(from.getData());
      dirtyProps.set(0, len); // TODO: nullProps?

      hydrator = from.hydrator;
      liveProps = from.liveProps;
   }

Running POC does not caused crashing any more.

And made some profiling on 7156 with and without changes from 6720.

1) clean 7156, number of times getData is called for running POC (warm and 1 test) is 1200000.
1) 7156 rebased with 6720, number of times getData is called for running POC (warm and 1 test) is 100000.

Majority of calls to getData are made to get data of a field of a buffer.

While I was going over the changes and trying to understand them, I tried a simple example that should have used the lazy hydration:

for each t2:
    message t2.camelCase.

In this case, all the fields are hydrated at the beginning instead of only the camelCase field when it is accessed. The reason for this is that when it gets to Session.getImpl it does loader.load which loads all the fields.

However, lazy hydration works fine when I have:

for each t2 no-lock:
    message t2.camelCase.

That's because when I have the no-lock, in ScrollableResults.get, it uses SQLQuery.hydrateRecord, which does the lazy hydration if the result set is a LazyResultSet.

In this case, all the fields are hydrated at the beginning instead of only the camelCase field when it is accessed. The reason for this is that when it gets to Session.getImpl it does loader.load which loads all the fields.

We might need to have this implemented for your first example as well, but lets go with small-steps and ensure the current 6720a is right. We might even think of a two iterations approach and handle the Loader.load case in the second iteration of the changes. My wish is to have this solution documented, "refined", fixed of problems and heavily regression tested. We need to get it in a stable position so we can do some statistics: how often does the optimization kick in (I think there are some MBeans for that), what is the performance upgrade, etc.

My current focus for now is to check if the rollback functionality works when the lazy hydration is in place. Unfortunately, while doing this, I found even more places when lazy hydration isn't used:

find first t2.
do transaction:

    t2.camelCase = 10.
    message 't2.camelCase' t2.camelCase.

    undo.
end.
find first t2.

message 't2.camelCase' t2.camelCase.
message 't2.camelCase' t2.PascalCase.

In this case also, all the fields are hydrated. My current focus, currently, is to find an example where the lazy hydration is in place without a lock, so I can modify the value for a field and then trigger a rollback.

Andrei, even though it is not that prominent for __temp database, I think it is still applied if I am not mistaken. If you do the same test, but with __temp tables (which don't do any locking), will it replicate?

I actually just managed also with persistent tables:

find first t2 no-lock.

find first t2.
do transaction:

    t2.snake_case = 10.
    message t2.snake_case.    

    undo, leave.
end.
find first t2 no-lock.

message 't2.snake_case' t2.snake_case.
message 't2.PascalCase' t2.PascalCase.

Here, I first make a find-first with no-lock, which allows lazy hydration, but doesn't allow field modification, then another find-first without no-lock. As the record is already in the cache, it returns that one, so the fields are not hydrated. Then, in the do block, a rollback is triggered. However, even though the data array from BaseRecord is not fully filled, the rollback is performed successfully, as it only had to change back the value for the field that has been modified, which obviously was in data.

As BaseRecord.getData is only used in about 10 places, my current plan is to make some testcases to stress out as many usages.

I probably found a possible problem with the usage of getData in Record.setPropertyValues. The problem is that here, we update the value for a field, but DON'T update liveProps of the BaseRecord, as we do in BaseRecord.setDatum. However, it should be noted that this function isn't currently using in the FWD code, however as it is currently public it can be used in the converted code. So, I have modified the converted code of:

find first t3 no-lock.
message t3.f1.
find last t3 no-lock.
message t3.f1.

     RecordBuffer.openScope(t3);
         new FindQuery(t3, (String) null, null, "t3.recid asc", LockType.NONE).first();
         message((integer) new FieldReference(t3, "f1").getValue());
         Map<String, Object> propValues = ((BufferImpl) t3).buffer().getCurrentRecord().getPropertyValues();
         new FindQuery(t3, (String) null, null, "t3.recid asc", LockType.NONE).last();
         ((BufferImpl) t3).buffer().getCurrentRecord().setPropertyValues(propValues, (a,b) -> b);
         message((integer) new FieldReference(t3, "f1").getValue()); //with trunk: 1; with 6720b: 2 

Committed 6720b/14923. I have addressed the issue exposed in #6720-71 by changing the corresponding bit from liveProps when data is modified in Record.setPropertyValues.

• Persister.insert Here it is not checked whether all the fields are hydrated, however I haven't managed to get in a state where I have a partially hydrated record here and I don't see how it would be possible
• Session.getImpl Here getData is called on a record obtained with loader.load, so all the fields are hydrated.
• Session.merge It is only used in Persistence.merge which isn't further used anywhere else and is marked as deprecated.
• UniqueIndex.createKey Currently, there is a single path to reach it starting from Validation.validateMaybeFlush. However, before the call that leads to UniqueIndex.createKey, it calls checkMaxIndexSize, where all the fields are hydrated. So, I don't think there is a way to get to UniqueIndex.createKey without having all the fields hydrated.
• BaseRecord.copy Eduard investigated this and his proposed changes from #6720-63 should fixed the issues.
• BaseRecord.getData(PropertyMapper pm) This is used only in ImportWorker class, so I don't think it can cause any issues.
• RecordBuffer.getData(RecordBuffer rb) This is used only in a toString method, so it is safe.
• Record.setPropertyValues already discussed and fixed in #6720-71, #6720-72.

I finished rebasing 6720b to trunk. 🥳

I also done some light testing to be sure we haven't lost the lazy hydration in the way and it seems fine.

I think there are still some esthetic fixes to be done regarding some javadocs, but the next important step is regression testing followed by performance testing. I think that the initial regression testing can be light(1 or 2 large customer applications) in order to get faster to the performance testing to see if the changes really provide a big improvement or some other changes would be necessary. If everything works well, before merging we will obviously do a more extensive testing.

During regression testing of a large customer application I found some problems. Most of them have a LazyHydrationException because of com.goldencode.p2j.persist.PersistenceException: Cannot use a closed database session

Created a smaller testcase for this:

def buffer b1 for t3.

procedure p1:
    find first b1 no-lock.
    message b1.f1_____.
end.
run p1.
message b1.f2______.

However, the error only reproduces when session-lifespan attribute is set to -1 in directory.xml. Setting this attribute to -1 implies that when exiting the scope of procedure p1, the session is closed and not made reclaimable.

Currently, as I think the session should indeed be closed in this case, the solution is to catch the LazyHydrationException and refresh the session inside the hydrator, probably using the BaseRecord.refreshHydrator.

Currently, as I think the session should indeed be closed in this case, the solution is to catch the LazyHydrationException and refresh the session inside the hydrator, probably using the BaseRecord.refreshHydrator.

I thought this was implemented in 6720b though. Looking at the changes:

+                     record.refreshHydrator(session);

from Persistence should have refreshed the hydrator. In trunk, as you request a new session, all active buffers are iterated and their records are "reassociated" with the new session. In 6720b, the records also have their hydrator refreshed. With your example, I would like to find out why the code in Persistence is not enough.

In 6720b, the records also have their hydrator refreshed. With your example, I would like to find out why the code in Persistence is not enough.

That's true the call from Persistence$Context.getSession should be enough. However, getSession is called only once, when the query is first executed, and when f2 is hydrated, there is no call to getSession.

I currently have this change to Hydrator.java:

=== modified file 'src/com/goldencode/p2j/persist/orm/Hydrator.java'
--- old/src/com/goldencode/p2j/persist/orm/Hydrator.java    2025-02-06 14:52:07 +0000
+++ new/src/com/goldencode/p2j/persist/orm/Hydrator.java    2025-02-12 14:02:41 +0000
@@ -73,7 +73,7 @@
 import java.util.BitSet;

 import com.goldencode.p2j.jmx.*;
-import com.goldencode.p2j.persist.PersistenceException;
+import com.goldencode.p2j.persist.*;

 /**
  * This class stores a lazy result set to enable deferred/lazy (just-in-time) hydration of a DMO. One or
@@ -200,6 +200,19 @@
     */
    public void refresh(BaseRecord dmo, Long recid, BitSet partialFields)
    {
+      Session session;
+      if (sessionRef == null || (session = sessionRef.get()) == null)
+      {
+         throw new LazyHydrationException(recid, -1);
+      }
+      else
+      {
+         if (!session.isOpen())
+         {
+            PersistenceFactory.getInstance(session.getDatabase()).getSession();
+         }
+      }
+      
       Loader loader;
       if (loaderRef == null || (loader = loaderRef.get()) == null)
       {

This way, if the sessiong is closed, we force the refreshing of the session in the Hydrator. With this fix, the failing tests of the large customer application drops from 59 to 16.

1. Should the session really be closed when exiting the scope of the procedure in the example from #6720-75?
2. Improve the fix from #6720-79 to overcame the case when sessionRef holds a null and also some other things.

So, I have tried to understand if that session should really be closed. Tested with trunk and indeed the session is closed in the example from #6720-75. However, if I add an assignment in the main block: b1.f2______ = b1.f2______ + 1., it converts to:

   @LegacySignature(type = Type.MAIN, name = "6720.p")
   public void execute()
   {
      externalProcedure(Six720.this, TransactionType.FULL, new Block((Body) () -> 
      {
         RecordBuffer.openScope(b1);
         RUN_CALL_SITE_1.clone().run();
         b1.setF2(plus(b1.getF2(), 1));
         message((integer) new FieldReference(b1, "f2").getValue());
      }));
   }

   @LegacySignature(type = Type.PROCEDURE, name = "p1")
   public void p1()
   {
      internalProcedure(Six720.this, "p1", new Block((Body) () -> 
      {
         new FindQuery(b1, (String) null, null, "b1.recid asc").first();
         message((integer) new FieldReference(b1, "f1").getValue());
      }));
   }

As the call to p1 is inside a block with TransactionType.FULL, the session is closed when exiting the block from execute method.

Therefore, as the session is rightly closed, I think the solution is just to reopen one in refresh if in need, so I will work on improving the fix from #6720-79.

Agree with #6720-80. Doing updates will keep the session open as long as the transaction is open. If there is no transaction, it could simply rely on the stale records the buffers have already loaded. The downside is that read-only operations are now also requiring the session to aid hydration. This may change the session lifecycle.

• if the session closes normally, then we still have plenty of time to do this work. Also, I don't expect many records to "survive" till the very end and still need full hydration. I can't tell if this will negate the lazy hydration improvement, but it will help us doing "hacky" work of reopening a session just to hydrate a DMO "a bit".
• if the session closes due to an error, then things are quite tricky as we may not be able to use that session anymore ... what is happening in that case in trunk:
  

  do transaction:
     create b1.
     b1.f1 = 1.
     do transaction:
        b1.f1 = 2.
        find first b1 where trim(b1.b2) = ''. // do something to trigger a persistence exception - maybe close the DB connection from the debugger?
     end.
     message b1.f1. // 1 or 2
  end.

I won't worry about the session close with an error, because that case is quite fatal anyway.

So, I have tried the example from the last note where I closed the session before the commit in debugger. As a result, the PersistenceException is thrown, the transaction is obviously not committed, the logic that follows is not executed. However, it tries to execute all the logic once again, which works fine if I don't forcefully close the session.

Regarding, the idea to fully hydrate the records that are still loaded when the session closes. I also like it, I could also add a JMX counter there. This way we will really know if it happens very often or not.

Regarding, the idea to fully hydrate the records that are still loaded when the session closes. I also like it, I could also add a JMX counter there. This way we will really know if it happens very often or not.

PLease note that the original idea of the task in #6720-1 did suggest this. ;-)

Committed 6720b/15700 with the the implementation to hydrate all the unhydrated fields when the session closes.

Tested the same large customer application as in #6720-79 and I have 17 failing tests compared to 16. However, it may be a false negative. I will look into this tomorrow.

On the other hand, I added a counter when a record needs to be fully hydrated in this situation and got 1179 hits. IMHO, I was expecting a lower number. I think I would want to check exactly how many times we are getting in the situation where the session is closed and we want to hydrate a field and maybe if the difference to what I obtained here is huge, reevaluate the solution of opening a session.

It seems that only about 3% of the total hydrated fields are hydrated during the session closing, so I don't think it adds a big overhead. Therefore, we can probably go ahead with that approach.

Good statistic! Quick question: are the numbers cumulative for temp and persistent tables. If so, can we make a distinction of these please. Even if there are only 3% fields hydrated on session close, maybe these are 50% of the persistent one. My point is that the times for _temp are way way lower than persistent times in terms of hydration.

Alexandru Lungu wrote:

Good statistic! Quick question: are the numbers cumulative for temp and persistent tables.

Yes, the numbers are cumulative. I am working now on making the distinction.

Interestingly, all the records and therefore fields that need hydration when the session closes are only for persistent tables.

• the number of fields that are hydrated represents both the fields that were hydrated using the LazyResultSet while it was valid, but also the fields that were forcefully hydrated because the LazyResultSet was no longer valid and it had to use the loader, which hydrated all the remaining fields. I think I could also make the distinction between the fields that are hydrated using the LazyResultSet and the ones using the fallback loader. If it shows up that there are many fields that get to use the loader, it may be an indication to try and improve this.
• the total number of fields is increased with the number of fields when a Hydrator is created in createHydrator. I am not sure it gives me the accurate number of fields.

Andrei, for denormalized extent, there can be tables with 100s or more of fields, with maybe 90% of them from extent. Can you double-check how this behaves?

We have also discussed during the meeting, but I will also post here for reference. Each element of expanded extents is hydrated individually, so it should be ok.

On the one hand, the results are good because most of the hydration happens using the LazyResultSet.
On the other hand, I was expecting MUCH less need for the hydration. It seems that we need, at least during the unit tests for this large customer application, to hydrate over 94% of fields from persistent tables. Even though we don't need to hydrate 6% of fields, 1% of hydrated fields happen with the Loader which could offset that gain. Regarding this, maybe some profiling of the usages of getDatum would be useful to be done. Maybe we can improve not to call getDatum in some cases.

However, I think the next step should be getting to a fully functional solution. After this, I think we should focus on performance improvements.

It seems that we need, at least during the unit tests for this large customer application, to hydrate over 94% of fields from persistent tables. Even though we don't need to hydrate 6% of fields, 1% of hydrated fields happen with the Loader which could offset that gain. Regarding this, maybe some profiling of the usages of getDatum would be useful to be done. Maybe we can improve not to call getDatum in some cases.

This is very surprising. Does the application code really access 94% of the fields?

I suspect the application does not actually need 94%. My gut tells me this is related to something we are doing internally in the runtime (hopefully something that can be reduced). Just a hunch; I have absolutely no data to back up this statement ;)

Eric Faulhaber wrote:

My gut tells me this is related to something we are doing internally in the runtime (hopefully something that can be reduced).

I also hope this is the case. I would want to do some profiling for the getDatum method to see its top usages. However, first, I want to fix the remaining regressions in the tests from the large customer application, then move on to the performance improvements.

Please note that the unit tests are starting with an empty database, so all persistent data that is used is actually created by the test. I would expect the unit test to query the new data and assert most of its fields .... or not. My point is that unit tests show very particular and unrealistic use-cases that may result in bad performance of lazy hydration. I also agree that we need to check what is causing this massive percentage of 94%, but keep in mind that the road may lead to some assertions which are comprehensive.

While I was investigating the unit tests for that large customer application, I found out a scenario that somehow doesn't work with 6720b:

def temp-table tt1 
field f1 as integer.

create tt1.
tt1.f1 = 1.

create tt1.
tt1.f1 = 1.

message buffer tt1:find-unique("where f1 = 1").
message 'done'.

In OE it shows an UniqueResultException, however with 6720b it doesn't, but it does with trunk. It seems that somehow in 6720b, SQLQuery.uniqueResult misses the part where it is validated the uniqueness:

         if (rs.next())
         {
            close(); // ???
            throw new UniqueResultException("Result not unique for " + stmt);
         }

After adding this, the UniqueResultException is correctly thrown.

I actually see now why that if statement was missing. When we call rs.next(), the result set will be invalidated, so we'll have to hydrate all the fields. I will be thinking of another solution to check the uniqueness but without calling next.

The issue regarding the lack of validation of uniqueness have been solved by calling rs.isLast() and making the result set of type TYPE_SCROLL_INSENSITIVE.

Unfortunately, this uncovered something else. In SQLQuery.uniqueResult, we have the following flow regarding the Statements. Up until now, almost every call to session.unownResultSet(sql);, was followed by a session.ownResultSet(sql, rs);, only in critical situation, the second call wasn't made. However, after we add the check with !rs.isLast(), the call to ownResultSet will miss in the situation when the result is not unique.
This matters because we could have 2 consecutive calls to unownResultSet, without any call to ownResultSet. In unownResultSet, we try to close the Statement, if it is present in cache, however don't make sure if it is already closed or not. The problem is that 2 consecutive calls to close for UnclosablePreparedStatement will throw this error Can't check in a statement which wasn't yet checked-out. An easy fix would be, in unownResultSet to evict from cache, the statement, not only to close it. However, this is not trivial because
this method is used in clearResultSetCache:

         resultSetCache.entries().forEach((entry) ->
         {
            unownResultSet(entry.getKey());
         });

However, maybe it's worth to refactor this method and allow to remove while iterating.

FYI, ownResultSet and unownResultSet are things specific to #6720 effort - feel free to adjust their usage to fit your needs. The "owning" mechanism is designed to avoid memory leaks and closing the result-set at the right time.

For solving the issue presented in #6720-98 I have created a new method which evicts the ResultSet from the cache and is called from uniqueResult. I have committed this and also other small changes like the JMX counters in 6720b/15701.

Apart from that I have also committed Eduard's changes from #6720-33 in 6720b/15702. However, this could need further attention because of the snapshot usage.

With these changes, I still have 14 failing tests for that large customer application. However, most of them have the same error, so I hope fixing those regressions will work faster.

I found another case where 6720b works incorrectly:

FOR each t3 except (f2) no-lock:
    message t3.f1.
end.

   protected void checkIncomplete(int index)
   {
      if (checkState(DmoState.INCOMPLETE) && !readFields.get(index))
      {
         RecordMeta meta = _recordMeta();
         String tname = meta.legacyName;
         String fname = meta.getPropertyMeta(false)[index].getLegacyName();
         String msg = "Field " + fname + " from " + tname + " record (recid " + primaryKey() + 
                       ") was missing from FIELDS phrase";

         ErrorManager.recordOrThrowError(8826, msg, false);
      }
   }

Currently, readFields holds the fields that were hydrated. So, this method needs to be a little changed.

I think readFields should be re-thinked a bit, because it has quite a weird integration with liveProps - it may be even redundant. I think we should rather rework markIncomplete to set the readFields based on the row structure from the very start, and not rely for a very late readProperty to do the setting. Also mind that _getReadProps is being used and provides inconsistent outputs.

Alexandru Lungu wrote:

I think readFields should be re-thinked a bit, because it has quite a weird integration with liveProps - it may be even redundant. I think we should rather rework markIncomplete to set the readFields based on the row structure from the very start, and not rely for a very late readProperty to do the setting. Also mind that _getReadProps is being used and provides inconsistent outputs.

I don't disagree, but for context: my original (albeit unfinished) implementation of lazy hydration (which added liveProps) was layered on top of the existing FIELDS/EXCEPT implementation (which added readFields). My thought was to not let lazy hydration override the fields to be hydrated, if the application developer already had given this enough forethought to provide a FIELDS/EXCEPT clause. That being said, there probably is a better way to integrate these concepts, as you suggest.

1. In BaseRecord.getUnhydratedFields I don't use anymore the readFields. I just return the negation of liveProps
2. In BaseRecord.readProperty, don't set readFields as I don't think there is any case when this method is called and the bit corresponding to that field is false. So, that call would be redundant.
3. Fixed an infinite loop from OrderedRowStructure: Here the curr variable wasn't updated: for (int curr = fields.nextSetBit(0); curr < propOffset; fields.nextSetBit(curr + 1))

With this fix the unit tests of that large customer application show no regression. I will now move on to the FWD tests of another customer.

getLoadedFields

Isn't this equivalent to the already existing getProps?

In BaseRecord.getUnhydratedFields I don't use anymore the readFields. I just return the negation of liveProps

Is this right? Technically speaking, if you use FIELDS (f1, f2, f3) (out of 10 in total) and you have only f1 hydrated, then the unhydrated ones are (f1, f3) and the rest should not be considered unhydrated. Otherwise, BaseRecord.refresh will gather all other fields from the database, although they were not meant to be read.

In BaseRecord.readProperty, don't set readFields as I don't think there is any case when this method is called and the bit corresponding to that field is false. So, that call would be redundant.

Right, I agree.

Fixed an infinite loop from OrderedRowStructure: Here the curr variable wasn't updated: for (int curr = fields.nextSetBit(0); curr < propOffset; fields.nextSetBit(curr + 1))

Nice catch.

With this fix the unit tests of that large customer application show no regression. I will now move on to the FWD tests of another customer.

Nice!

Alexandru Lungu wrote:

getLoadedFields

Isn't this equivalent to the already existing getProps?

Well, I made getLoadedFields return a BitSet with loaded fields. This makes integration in markIncomplete seamless. Using getProps would have added another layer of complexity as I would have needed to get the index in the data array using something like dmoMeta.recordMeta.getIndexOfProperty.

Alexandru Lungu wrote:

Is this right? Technically speaking, if you use FIELDS (f1, f2, f3) (out of 10 in total) and you have only f1 hydrated, then the unhydrated ones are (f1, f3) and the rest should not be considered unhydrated. Otherwise, BaseRecord.refresh will gather all other fields from the database, although they were not meant to be read.

Yes, you are right, I will switch it back.

Andrei Plugaru wrote:

Yes, you are right, I will switch it back.

Done that in 6720b/15704.

Well, I made getLoadedFields return a BitSet with loaded fields. This makes integration in markIncomplete seamless. Using getProps would have added another layer of complexity as I would have needed to get the index in the data array using something like dmoMeta.recordMeta.getIndexOfProperty.

I see your point.

I have ran the FWD tests of another large customer application. Unfortunately, it shows some regressions :((

For persistent tables, we had to only hydrate less than 40% of the total number of fields. Also, when the session closes we are hydrating a small number of fields - about 1%.

I did some investigation of the error I get in the FWD tests. The good news is that it seems that all have the same error message, so I hope they all have the same root cause.

The error I get is Instance opened twice. We have actually encountered this exact error a few months ago also on the tests from that customer. The task is #9154. Even though the context and the root causes are probably different, during that investigation I also looked at this call in RandomAccessQuery.executeImpl: Record placeholder = referenceRecord != null ? referenceRecord : buffer.getSnapshot();. For that task, this turned out to be a false lead. However, I got immediately triggered when I saw the getSnapshot call as it is a known issue currently for lazy hydration.

Alex, we also discussed during the meeting it may be more efficient to first try to solve that snapshot issue. Based on the above paragraph there is a non-zero probability, the current regressions are because of this. However, even if they turn out not to be, it will be good to have that issue solved.

I remember task #9030 being mentioned in a meeting as having a potential solution for this, I checked out branch 9030b, and saw that getSnapshot is no longer used in RandomAccessQuery.executeImpl.

My question is: should we pause the work on this task and focus in getting 9030b finished and then resume work here?

Andrei, having 9030b in trunk will be ideal, but I don't have enough focus and time to continue work onto that :/
Please switch focus entirely to #9030. We can discuss the deal there before. I think #9030 is indeed a prerequisite of #6720.
I will follow your progress closely on #9030.

I am further investigating the regressions on the FWD tests. I still think that the root issue could be related to the snapshot usage. The solution to fully hydrate there is unacceptable as it gets there too many times.

I have an example where the snapshot of the unhydrated record could cause some potential issues:

def buffer b1 for t3.

find next t3 where f1 > 1 no-lock.
find next b1 where b1.f1 > 1 no-lock.
buffer-copy b1 to t3.
find next t3 where t3.f1 > 1 no-lock.
message t3.f1_____.

In the last find next query it has as placeholder an unhydrated record. I think this could be prone to errors if the hydrator is unvalidated and has to reach again to the DB which could have different data than it had initially.

In the last find next query it has as placeholder an unhydrated record. I think this could be prone to errors if the hydrator is unvalidated and has to reach again to the DB which could have different data than it had initially.

True. Can you pinpoint the exact flow here (the stack trace when the snapshot is done)? Also, please clarify what snapshot do you mean: RecordBuffer.snapshot or BaseRecord.snapshot()?

Starting from the testcase in the last note, Alex and I analyzed more RecordBuffer.armWriteTrigger method.

We analyzed the option to fully hydrate(or hydrate only the fields that are used in indexes) the record in some cases. Unfortunately, this is probably is not a viable solution. In RecordBuffer.armWriteTrigger we use the snapshot as the old buffer. Therefore, we need to have the exact state of the buffer at that moment, if it somehow gets a different data from the DB, would be a big mistake. This option for write triggers is used pretty often by a large customer application. However, an even bigger problem is the Generic Triggers which is used by another large customer application and for all tables(I think). This would mean that full hydration would be necessary always. The other option - to hydrate only the fields that are used in indexes , is also not right because each field of the old buffer can be accessed, not only the ones that are in indexes.

Andrei, I just found another usage of snapshot. A so called armCurrentChanged that can be triggered if you run a FIND CURRENT tt. In this case, FWD will keep a snapshot of the current record and attempt to load it again from the database. After, updateCurrentChanged is meant to check if the new record from the database is different from what was snapshot. In this case, updateCurrentChanged is going to iterate all fields anyway to do the comparison, so I think this kind of fully hydration is acceptable in order to satisfy updateCurrentChanged logic.

Are we adding implicit accesses (in our runtime) to fields that are not otherwise accessed explicitly in the 4GL code?

On its face, I don't see why we would otherwise need to hydrate for the buffers passed to triggers. In other words, we shouldn't care if a buffer has been hydrated before firing a trigger because any triggers being fired are known to still be in the scope of the related buffer being processed elsewhere in the application. If we access fields during the trigger, they would need to be hydrated at that time. If the original buffer has already been edited, then those fields will already have been hydrated and modified.

Are we adding implicit accesses (in our runtime) to fields that are not otherwise accessed explicitly in the 4GL code?

Well, "snapshot" process that we do in FWD run-time can be considered "implicit access" that is not triggered explicitly from 4GL code. This is done before changing for the first time a freshly loaded record from a buffer. The goal is to satisfly a future TRIGGER PROCEDURE FOR WRITE OF table OLD BUFFER old. As Andrei mentioned, old buffer is the problematic construct.

On its face, I don't see why we would otherwise need to hydrate for the buffers passed to triggers. In other words, we shouldn't care if a buffer has been hydrated before firing a trigger because any triggers being fired are known to still be in the scope of the related buffer being processed elsewhere in the application. If we access fields during the trigger, they would need to be hydrated at that time. If the original buffer has already been edited, then those fields will already have been hydrated and modified.

This affirmation is correct for the "new buffer", but not for the "old buffer". The old buffer stores a snapshot of the record before being changed for the first time. This very snapshot is read-only, but should be fully hydrated. We can't keep a live hydrator for it because its data is intentionally stale.

Creating the snapshot of a partially-hydrated record will take only the information available at that time. I do not remember the exact code, but that R/O instance does not take into consideration the missing properties (Record.snapshot()). This must be implemented.

OTOH, the record is R/O at 4GL level, it can be updated backstage if a missing property (which was not needed in normal 4GL code) is required by the trigger. So theoretically, the record can be updated to provide the original (and unchanged) properties which were missing. The problem is when to do that? It might not be performance-wise optimal to request each property from SQL each time one is needed in the trigger: that will cause multiple SQL accesses if multiple needed properties were not hydrated. Probably we'll have to do a full hydration of the record at the moment a missing property is accessed during the execution of the trigger. I thought of other variants but this seems like the best solution.

Probably we'll have to do a full hydration of the record at the moment a missing property is accessed during the execution of the trigger.

The problem with this approach is that, we will get the current data from the database which could be different than the data when the snapshot was taken.

This affirmation is correct for the "new buffer", but not for the "old buffer". The old buffer stores a snapshot of the record before being changed for the first time. This very snapshot is read-only, but should be fully hydrated. We can't keep a live hydrator for it because its data is intentionally stale.

Isn't the old buffer still in scope in the original 4GL code whose processing caused the trigger to fire? If so, then why do we need to fully hydrate? If not in scope, how does that happen? It seems like a triggers would fire before an associated buffer is out of scope.

The 'old' buffer is not a normal buffer. The snapshot of a record is created at the time it is changed (only if a trigger is also identified for it). It is not stored in a buffer. However, one is manufactured as a read-only at the moment the trigger is fired. It is not bound to any block as normal buffers and we only mimic the minimal scope to that its content could be read.

The problem I foresee is that at the moment the snapshot is created, the record might not be complete (fully hydrated), causing the values of the fields seen in trigger to be incorrect (usually null/unknown).

The 'old' buffer is not a normal buffer. The snapshot of a record is created at the time it is changed (only if a trigger is also identified for it). It is not stored in a buffer. However, one is manufactured as a read-only at the moment the trigger is fired. It is not bound to any block as normal buffers and we only mimic the minimal scope to that its content could be read.

I understand. I presume it is scoped to the trigger. It is implicitly nested inside the converted 4GL block current executing, which did something that caused the trigger to fire.

There is an "original" application buffer somewhere in the enclosing scope of this trigger execution. That original buffer holds a record queried from the database which is the subject of the trigger and some number of edits of that original buffer may have been made. We provide the "old" buffer which the trigger can use to compare against the new buffer which is either the same as or a a copy of the original buffer.

Do I understand correctly?

If so, my point here is that we still have full access to the original buffer and its JDBC result. Why do we need to force full hydration here? We can just lazy hydrate as needed since the JDBC result is still there. I'm assuming that when any edits are made to the original buffer, that certainly means that we hydrated those fields that were updated and then when there was an assigment to one or more of those fields we can just copy the original value into the snapshot but leave the rest of the snapshot sparse. We don't want to force hydration of the entire snapshot and I don't understand why it would be needed.

The problem I foresee is that at the moment the snapshot is created, the record might not be complete (fully hydrated), causing the values of the fields seen in trigger to be incorrect (usually null/unknown).

Downstream of the usage of the original buffer, if a trigger fires and the trigger's code accesses some unedited field then it will force hydration on that field. If it accesses an edited field, the edited value would already be known and the snapshot of the original value would already be known so there is no hydration.

I may be misunderstanding things but it seems like we do a lot of forced hydration when we could leave it lazy. As long as the JDBC result is still in scope we should maximize our laziness.

Greg Shah wrote:

I understand. I presume it is scoped to the trigger. It is implicitly nested inside the converted 4GL block current executing, which did something that caused the trigger to fire.
There is an "original" application buffer somewhere in the enclosing scope of this trigger execution. That original buffer holds a record queried from the database which is the subject of the trigger and some number of edits of that original buffer may have been made. We provide the "old" buffer which the trigger can use to compare against the new buffer which is either the same as or a a copy of the original buffer.
Do I understand correctly?

That is correct.

If so, my point here is that we still have full access to the original buffer and its JDBC result. Why do we need to force full hydration here? We can just lazy hydrate as needed since the JDBC result is still there. I'm assuming that when any edits are made to the original buffer, that certainly means that we hydrated those fields that were updated and then when there was an assigment to one or more of those fields we can just copy the original value into the snapshot but leave the rest of the snapshot sparse. We don't want to force hydration of the entire snapshot and I don't understand why it would be needed.

Indeed, from the JDBC result we need to (and we actually can) lazy hydrate the snapshot, as the fields of the 4GL buffer are lazy hydrated. Something like this:

Event:    fetch(r)          modify(r.x r.y r.z)    modify(r.b)        modify(r.a)      flush(r)
JDBC:     x y z a b c       x y z a b c            x y z a b c        x y z a b c      P Q R A B c
4GL:      x y z . . .       P Q R . . .            P Q R . B .        P Q R A B .      P Q R A B .
snapshot: (null)            x y z . . .            x y z . b .        x y z a b .      (null)

Currently the snapshot is not (from my knowledge) mutable. We do not have the information of what fields are currently hydrated in this backstage copy. So we need some kind of bitfields to mark them, and avoid a double initialisation with an intermediary value.

However, I think we have the JDBC to create the old buffer directly, at the moment the trigger is called instead of creating the intermediary snapshot. So the old buffer would be a fully 'dry' record which will be lazy hydrated during the execution of the trigger only for (and if) a specific field of the old buffer is accessed.

However, I think we have the JDBC to create the old buffer directly, at the moment the trigger is called instead of creating the intermediary snapshot. So the old buffer would be a fully 'dry' record which will be lazy hydrated during the execution of the trigger only for (and if) a specific field of the old buffer is accessed.

Mind that the original solution to this is already in #6720-63 (which does the snapshot, but also attaches the hydrator that wraps the JDBC result-set). The liveProps are bound to the record, so one hydrator can hydrate multiple records. We have the technique here. Please refer to #6720-23 for the design presentation.

However, the Hydrator is quite volatile as it weakly references the session and other persistence objects. So technically speaking, the Hydrator can end up with an invalid JDBC result-set underneath and is forced to refresh. The refresh is going to spoil the snapshot - this is the problem. Currently, the result-set is invalidated when committing/rollback-ing the transaction, closing or using next (or other "moving" API). Maybe a good safe guard is to "freeze" the hydrator and not allow refresh for "snapshot" cases. If it does so, log or throw conditional exception. But will be unfixable; it will only notify us of the bad design we approached in the first place.

1. A quite weird scenario:
   

   OPEN QUERY q FOR User.
   GET FIRST User. // LAZY HYDRATING User
   User.name = "test".
   RELEASE User. // fire WRITE trigger

   
   and the trigger:
   

   // WRITE TRIGGER
   // get QUERY q somehow (GLOBAL SHARED or whatever)
   GET NEXT q. // hydrator is invalidated

1. I am not quite sure how TRANSACTION-MODE AUTOMATIC. can interact with this. Can we commit or rollback inside a WRITE trigger? I think this statement is worth exploring either way, because we don't want to mess up the lazy hydrator with TRANSACTION-MODE AUTOMATIC..

1. IIRC, FIND FIRST User. is lazily hydrated, but the result-set is bound to the prepared statement executed. If we do a second FIND FIRST User2., the cached prepared statement will execute again, invalidating the old result-set, so while User2 will be lazily hydrated, User looses its hydrator. For snapshot case.
   

   FIND FIRST User.
   User.name = "test".
   RELEASE User. //fire WRITE trigger

   
   and the trigger:
   

   // WRITE TRIGGER
   FIND FIRST User2. // hydrator is invalidated

PS: there are very few refreshes usually, but it is a must to have them in order to recover from invalidated hydrators.

The original implementation of #6720 was to cover as many scenarios as possible, but the OPEN QUERY and FIND FIRST cases are showing vulnerabilities exactly for this snapshot cases. I mentioned this early in the design phase #6720-21: such queries can lose the records "in the wild". If we want to have snapshots for OLD BUFFER, then we need to drop support for FIND and OPEN QUERY and stick only to FOR EACH cases.

• 16110 and 16112: Apart from unimportant rebase issues, I have changed the implementation of BaseRecord.equalData. This method has been introduced in #9701 in order to have a faster DMO equality check. Up until now, this method was just checking if the elements from the data arrays are equal. This idea, however, doesn't work anymore when lazy hydration is in place. So, at first, in 16110 I was just calling getDatum for each field. Then, in 16112, I have made the logic a little smarter, by executing the fast array equality check if all the fields are hydrated, and fallback to calling getDatum on each field, otherwise.
• 16111 contains more changes. First, I had some problems in FullRowStructure because of the expanded mode for extents. So, currently in FullRowStructure.hydrateAt, I am hydrating the entire expanded extent if the offset is on one. However, this could be improved by hydrating only that exact element from the extent. I have still not implemented that because I cannot figure it out how to get the offset in the result set for that exact element. I will try, however to come up with an idea for this. Next, I encountered another issue because of Session.getImpl. Here, when a refresh for a record was needed, a new dmo was loaded with data from the DB, and its data was just copied to the already existing dmo. The hydrator for that dmo remained the same. The problem was when we tried to hydrate a field which wasn't in the original row structure. This was failing with Can't hydrate property that was not selected into the result-set.. My solution was to set liveProps to true for all fields. I think this is correct as, in that scenario we already have all fields hydrated, so there is no need in trying to hydrate that again.

With all these changes, I have run the harness tests of a large customer application and it passes. My plan is to also run the unit tests of a large GUI application as I know they were passing last time I worked on this task. If they turn out to be successful, I will get back to the known issues described in the last notes. I would want to use Alex's idea: to drop support for FIND and OPEN QUERY and stick only to FOR EACH cases.

The original implementation of #6720 was to cover as many scenarios as possible, but the OPEN QUERY and FIND FIRST cases are showing vulnerabilities exactly for this snapshot cases. I mentioned this early in the design phase #6720-21: such queries can lose the records "in the wild". If we want to have snapshots for OLD BUFFER, then we need to drop support for FIND and OPEN QUERY and stick only to FOR EACH cases.

I'm not OK with limiting this implementation to only FOR EACH. Large percentages of application queries would be missed in this way.

I don't understand the problem with the OPEN QUERY or FIND FIRST examples in #6720-126. In both of them, the problem occurs during a RELEASE. By definition, in the 4GL code you can no longer access that record's contents after that moment. It is the equivalent of exiting/iterating the scope of the buffer at an arbitrary point in the code. After that, we should not expect to access the JDBC result set, nor would any hydration be needed since the record is "gone".

Please help me understand.

16110 and 16112: Apart from unimportant rebase issues, I have changed the implementation of BaseRecord.equalData. This method has been introduced in #9701 in order to have a faster DMO equality check. Up until now, this method was just checking if the elements from the data arrays are equal. This idea, however, doesn't work anymore when lazy hydration is in place. So, at first, in 16110 I was just calling getDatum for each field. Then, in 16112, I have made the logic a little smarter, by executing the fast array equality check if all the fields are hydrated, and fallback to calling getDatum on each field, otherwise.

Are you saying that we force hydration of all fields any time BaseRecord.equalData is called? Is that needed?

Part of my worry about this task is that we have areas of our persistence implementation which drive extra hydration when it is not really needed. This could be the case because we implemented many features (e.g. layers of caching) in a way that may have assumed all fields are already hydrated. In a worlds where we are trying to maximize how lazy we can be about hydration, we may need to rework areas of persistence rather than accept limitations.

My rule: if the 4GL would require the field to be accessed, then it should be hydrated. If the field would not be accessed in the 4GL, then it should not be hydrated. Caches, snapshots... whatever should all be sparse objects whenever possible.

Greg Shah wrote:

Are you saying that we force hydration of all fields any time BaseRecord.equalData is called? Is that needed?

Yes, but this was actually the case even before #9701. The return value of the equalData method is used to update the recordChanged variable in RecordBuffer. As far as I understand, recordChanged should really tell us if the record we have matches the one from the Database, which can't be done without hydrating all the fields. This is used in the
P2J implementation of the CURRENT-CHANGED method.

I agree that there could be cases when we are doing field hydration even though we don't necessarily need it, however I don't think it is the case for BaseRecord.equalData.

I'm not OK with limiting this implementation to only FOR EACH. Large percentages of application queries would be missed in this way.

I think you are right. Currently the implementation is for FIND, OPEN QUERY and FOR EACH, so lets try to achieve our goal this way.

I don't understand the problem with the OPEN QUERY or FIND FIRST examples in #6720-126. In both of them, the problem occurs during a RELEASE. By definition, in the 4GL code you can no longer access that record's contents after that moment. It is the equivalent of exiting/iterating the scope of the buffer at an arbitrary point in the code. After that, we should not expect to access the JDBC result set, nor would any hydration be needed since the record is "gone".

• the old buffer stores a snapshot record that eventually has hydrator B. This hydrator B relies on the result-set provided by the OPEN QUERY q FOR User.
• the new buffer stores a cached record that eventually has a hydrator A.

I am quite sure that A = B.

• the new buffer that holds the cached record can do that, because the DMO is cached and shall be a live representation of what is inside the database + changes inside memory that are represented already in the hydrated fields anyway.
• I am more stressed about the old buffer that holds the snapshot record, because the database content may be newer than the actual snapshot.

The more I think about it, this may not be a problem after all. The database can't have newer contents than the snapshot inside the WRITE trigger, mostly because the record wasn't flushed yet. So simply re-reading the record from the database will get us a proper snapshot. As for concurrency, another session can't update the data of that record because it should have had an EXCLUSIVE-LOCK in the first place. I wonder if there is a blind spot here.

Please ignore my concern for now. I will think it a bit more through.

I agree that there could be cases when we are doing field hydration even though we don't necessarily need it, however I don't think it is the case for BaseRecord.equalData.

I wonder if we can refactor FIND CURRENT to rely more on the sharedVersion and version. So instead of refetching the record from the database, we can check if the record is simply STALE and confirm that in that case it is also CURRENT-CHANGED. There are also some blind spots:

• whether the record was simply touched by another session ... does it mean that is also changed?
• if the DMO loaded is a dirty copy.
• if the DMO was deleted in the mean-time by another session.

Thanks Alex for the comprehensive overview. I will think about the refactoring of FIND CURRENT and maybe others instances of field hydration that could be avoided.

However, first, I would want to get to a stable version where tests of multiple clients completely pass. Currently, I am still investigating some regressions with the current implementation.

Yes, but this was actually the case even before #9701. The return value of the equalData method is used to update the recordChanged variable in RecordBuffer. As far as I understand, recordChanged should really tell us if the record we have matches the one from the Database, which can't be done without hydrating all the fields. This is used in the P2J implementation of the CURRENT-CHANGED method.

I assume that at any point in time, we can answer the following questions for ANY buffer that currently has a record loaded:

1. Which fields are hydrated? I assume we track this with BitSet or equivalent. This will be the set of fields that have been read or written since the query returned the record.
2. Which fields have been changed? Again, this seems like a candidate for a BitSet. This will only be the set of fields where data has been changed. I think we might even ignore assignments to the field that don't actually change the value but I don't know if that breaks 4GL behavior.
3. Have these changes been flushed to the database?
4. Have the changes been committed?

With the above information, we might be able to answer CURRENT-CHANGED without full hydration. In fact, don't we already know the answer here?

We should check in the 4GL, but the docs for CURRENT-CHANGED associate it exclusively with a FIND CURRENT on that buffer. It seems to me that we should calculate (or really just "save") the answer to CURRENT-CHANGED at the moment we do a FIND CURRENT and CURRENT-CHANGED just reports the output of that saved flag. If we find some aspect of the 4GL behavior that cannot be duplicated from our existing in memory state (the questions above), then we can limit the full hydration to that FIND CURRENT case ONLY.

What I'm looking for is for us to push the boundries of what is possible without hydration. We know that all hydration is 100% overhead that FWD has which OE does not have. In some cases, that overhead has been measured at 5% of a procedure's CPU usage, so it can be a real penalty. The time for naive/simple implementations of hydration is long gone.

1. FullRowStructure.hydrateAt: made it hydrate only the needed element from the extent by calculating the current extent index in the result set.
2. SQLQuery.hydrateRecordImpl: improved the case of the INCOMPLETE cached record in order to reduce calls to Session.get. Instead of calling Session.get(which would have implicitly hydrated all the fields), I have made it attach a new hydrator with the needed fields.

With the latest commit (6720b/16114) there are no more problems in the large GUI application. So, I will continue testing other applications. Last time I worked on this task, there were issues in the FWD tests of a large application(#6720-110). However, currently, the process of testing it is not straightforward, as the custom branch they are using got out of sync with the trunk, but I will think of some workarounds to test it.

Then, after the functional issues will be gone, I will get to performance side by trying to reduce the need of hydration(I could start with the ideas exposed in #6720-132, #6720-134).

1. ChangeSet.java This class stored a copy of the data array, which is further used in the rollback process. This works fine for the 1st field update, as the old testcase from #6720-69 showed no issue. The problem occurs when the next fields are updated. That is because we take a snapshot of the data array before the 1st field update, but after hydrating that field. So, the array in ChangeSet contained the right value for the 1st updated field. At further updates, the snapshot array is not modified, so it had unhydrated values for the other fields. In order to fix this, I am modifying the array from ChangeSet at each field update(with the old value, of course). I am also keeping a BitSet in order to ensure that we are changing the value only one time, before the 1st update. Apart from this change, here I have also replaced some raw access to the data array with setSimpleDatum in order to modify the liveProps bitset.
2. BaseRecord.copy This method had an important mistake regarding the liveProps bitset. We were assigning the same object, which made that hydrating a field in one of the records made it think that field was also hydrated in the other one. This resulted in returning unhydrated values. Apart from that, also added the assign for livePropsCardinality as it was previously missing.

I have good news and bad news:))

I have tested the fwd tests and unit tests of another large customer application and there are no regressions with the current revision. LE: Actually, while analysing the results closer, there are a few regressions, however the solution for most of them is simple.

On the other hand, I have tried to stress out more the implementation, especially when the hydrator is invalidated. Unfortunately, I have found a testcase which gives the wrong result when it has to fallback to load the record from the DB.

Procedure 1:

DEFINE VARIABLE qh AS HANDLE.
DEFINE BUTTON btS LABEL "next".
DEFINE FRAME f
 btS.
def buffer b1 for test6_8971.
CREATE QUERY qh.
qh:SET-BUFFERS(BUFFER b1:HANDLE).
qh:QUERY-PREPARE("for each b1 WHERE f1 > 1 no-lock").
qh:QUERY-OPEN.
qh:get-next. 
qh:get-next. // 2nd record is retrieved and has a LazyResultSet associated
qh:get-next. // 3rd record is retrieved

qh:get-prev. // back to the 2nd record, however at this stage the result set is invalidated as moving operations have been performed
// now, procedure 2 is run, which updates the record
on choose of btS IN FRAME f do:
    message b1.f1 b1.e1.// because the result set is invalidated, we have to load from the DB, which will return the modified record from procedure 2; as a result, b1.e1 will be 1000 instead of the initial value
end.

ENABLE btS WITH FRAME f.

WAIT-FOR CLOSE OF CURRENT-WINDOW.

Procedure 2:

find first test6_8971 where test6_8971.f1 = 3 no-error.
test6_8971.e1 = 1000.

The root cause is the fallback to Loader which causes in retrieving a newer state of the record than we would have expected. I will try to think of some possible solutions to this.

The root cause is the fallback to Loader which causes in retrieving a newer state of the record than we would have expected. I will try to think of some possible solutions to this.

The OE implementation is subject to READ_COMMITED transaction isolation level. I think this is not a problem. If you pick up a record with NO-LOCK, there is no guarantee of the data you are going to see. Seeing data from another committed transaction is totally fine. If you would wanted to have proper access to the record, you should have used SHARE-LOCK or EXCLUSIVE-LOCK.

However, these is pitfall here: what if there is an UNIQUE index with two columns (a, b). You fetch first a, the second user modified and index and later on you fetch b? This will make the first session to see a "mixed" group of values of that unique index (e.g. (a = 1 and b = 2), where the OG values were (1, 1) and later transformed into (2, 2) by the second session). Technically, you can't "see" unvalidated updates from one session to another in OE, not even with cross-session dirty-share quirk.

My suggestion is to forcefully fetch other unique index components when accessing a uniquely indexed field (e.g. hydrating "a" will also hydrate "b"). Hopefully, this would not be much of a performance hit.

• I have added the implicit hydration of fields from unique indexes proposed by Alex in #6720-138
• I have rebased the branch to a current trunk
• I have functionally tested 3 large customer applications and found no issue

• a converted 4gl for each block that iterates over all records, but doesn't access any field. So, with 6720b, we avoid hydrating 100_000 fields. Here I have measured the execution time of the entire for each block.
• a simple Java program that executes a SELECT query over that table, and iterates over the entire result set. In order to simulate the hydration I have called getString for each field. In FWD there is more logic where hydrating, but I think this is the most time consuming.

The results were surprising. For the 4gl test, the difference in execution time was really small(<5 ms). Also, about the same time difference I was getting with the Java program. However, after a talk with Alex, I added more data in the table in order to simulate a more realistic scenario. I populated the table still with 2000 records, but each field has a 1000 character long random generated string. Even though the total execution time is higher, the difference between lazy hydration and trunk still remains pretty small. For the 4GL test, the difference between 6720b and trunk is pretty small(<20ms). I have also observed a similar time difference during the simple Java program(about 20ms). I understand that in some real life scenarios, there could be situations where hydrating a field would consume even more time, but I think a scenario with 1000 character long string is still above the average complexity.

To be noted that, the examples I have made represent a best case scenario, as I only took into account a case where we have the hydrator available. The degradation that was observed in application probably comes from reloading the record from the DB because of the invalid hydrator. An improvement of dozens of ms is easily shadowed by a few DB queries which can take 10s of ms each.

At this moment, even though I am a little dissapointed, I think there can be some things that can be done. First of all, as far as I understand, at the beginning of this task profiling has been done and hydration proved to be worth optimising. So maybe, there are some other cases when hydration is more time consuming and maybe some heuristic can be done that will only apply the lazy hydration on these more consuming ones. Furthermore, maybe we can also try and get rid of the reloading logic. I just found the CachedRowSet(https://docs.oracle.com/javase/8/docs/api/javax/sql/rowset/CachedRowSet.html) which should basically cache one or more rows in memory. This is definitely less efficient that lazy hydration as there is still logic being executed to copy the low level data, but it is still better than also executing the additional FWD logic and also, it should be more performant than making another SQL query. However, more research should be done here in order to understand if it doesn't cause other problems(e.g. memory leaks).

The conclusion is that a getter call over the result set takes more time h2 vs a persistent table.
Similar results were also obtained for a application that used PostgreSQL. More interestingly, most of the time for org.h2.jdbc.JdbcResultSet.getString is not spent for retrieving the data per-se, but rather for some check operations: checkColumnIndex, checkOnValidRow, traceDebugCall. As we have full control over H2, I am wondering if these operations can somehow be improved.

Based on the previous note, it seemed that hydrating a record from H2 would be slower than a persistent one. So, I went ahead and created a Java program which used a H2 DB, and timed how much it was taking to iterate over the result set and call a getter on each field.

The setup is as for the previous notes: 50 fields; 2000 records; each field had as value a 1000 character long string. The results, however, are a little strange, compared to the one from the YK snapshot. It was taking less than 10ms to iterate over the result set and call getString on each field. I can speculate that the reason the results are the other way around is that the hardware the tests is running is much slower or the profiling impacts some methods differently.

At this moment, as the actual time taken to call the getters seems very low, and as with the current implementation we always have the risk of making additional SQL queries(which can have a big impact on performance), I don't really think the actual implementations can have great performance results(maybe worse if there are a lot of records reloads). I understand that Alexandru saw a slight performance improvement in #6720-40, #6720-41, however, maybe in the mean time, JDBC drivers improved some logic for the getters or that scencarios had few records reloading.

However, maybe it is worth investigating a techinque to actually only fetch from the DB the fields we are using in the runtime. I see there already exists #6721 and #8067. This has the benefit of less data being sent over the network, however, there are also the drawbacks highlighted by Alex in #8067-17.

In these tests you aren't measuring the cost of creating the BDT values. That tends to be expensive.

Greg Shah wrote:

In these tests you aren't measuring the cost of creating the BDT values. That tends to be expensive.

Well, I don't think lazy hydration really avoids creating BDT objects. Lazy hydration just populates the data array from Record on demand. However, in this array we store Java base types. The BDT objects are created in the getters from Record based on the values from the data array. However, lazy hydration shouldn't reduce the number of calls to these getters.

Well, I don't think lazy hydration really avoids creating BDT objects. Lazy hydration just populates the data array from Record on demand. However, in this array we store Java base types. The BDT objects are created in the getters from Record based on the values from the data array. However, lazy hydration shouldn't reduce the number of calls to these getters.

Indeed, Record.data stores raw Java values that are eventually wrapped in BDT types when we use get (or unwrapped when we use set).

Alexandru Lungu wrote:

Well, I don't think lazy hydration really avoids creating BDT objects. Lazy hydration just populates the data array from Record on demand. However, in this array we store Java base types. The BDT objects are created in the getters from Record based on the values from the data array. However, lazy hydration shouldn't reduce the number of calls to these getters.

Indeed, Record.data stores raw Java values that are eventually wrapped in BDT types when we use get (or unwrapped when we use set).

#9060-53 is relevant for this. Overall #9060 promotes caching immutable BDT on record getters to avoid creating new instances each time we do a get over a DMO. But this can apply only to immutable BDT. From #9060:

Save the BDT to Record level lazily using a separate array to store them. Consider invalidation when necessary (when altering data due to setters).

Testcase where the Session.associate error appears (item table has at least one record):

FIND FIRST item NO-LOCK.

FOR EACH item NO-LOCK:
    DISPLAY item.
END.

Alexandru, what do you think we should do in this scenario:

DEFINE VARIABLE number_item AS INTEGER NO-UNDO.

IF CAN-FIND(FIRST item NO-LOCK) THEN DO:
    FIND item NO-LOCK.
    number_item = item.itemNum.
    MESSAGE "The item number is: " number_item VIEW-AS ALERT-BOX.
END.
ELSE
    MESSAGE "No items found." VIEW-AS ALERT-BOX.

CAN-FIND(FIRST item NO-LOCK) returns true because there is a item record in database. Since is a idOnly query, sets the record as INCOMPLETE and saves it into Session cache.
On FIND item NO-LOCK., SQLQuery.hydrateRecordImpl gets called and fetches the record from Session cache. It will check if the DMO is INCOMPLETE, which will then check the read fields and needed fields and leave the buffer as it is.
When the field getter is being called, it will raise error Unable to update item Field. (142).

For this situation, what do we think? It is safe to set this record as complete and set newHydrator to true when we have FullRowStructure?

Why would we ever hydrate or store state for a CAN-FIND? It should not affect any buffers (that are explicitly referenced by 4GL code) so it should be outside of the caching and hydration implementation.

Greg Shah wrote:

Why would we ever hydrate or store state for a CAN-FIND? It should not affect any buffers (that are explicitly referenced by 4GL code) so it should be outside of the caching and hydration implementation.

We should not hydrate CAN-FIND, but 4GL (and FWD trunk) "hydrates" FIND FIRST/UNIQUE queries.
The point is that the INCOMPLETE state from CAN-FIND propagates through that FIND item NO-LOCK. and makes it to fail on itemNum field getter.

Obvious, the CAN-FIND is not the only case where this scenario can happen:

FOR EACH item FIELDS (itemName) NO-LOCK:
   DISPLAY item.itemName.
END.

FIND item NO-LOCK.
number_item = item.itemNum. // ERROR
MESSAGE "The item number is: " number_item VIEW-AS ALERT-BOX.

The point is that the INCOMPLETE state from CAN-FIND propagates through that FIND item NO-LOCK. and makes it to fail on itemNum field getter.

This is a bug. Records from the session cache that are incomplete can't be used to resolve other queries that require more fields. I thought this was fixed in the past. From Session.get:

if (dmo.checkState(DmoState.INCOMPLETE) &&
             (partialFields == null ||
              !isSubset(partialFields, dmo.readFields)))
         {
            // we have a cached record which is incomplete, and this requires to refresh the incomplete record
            // from the database
            refreshIncomplete = true;

            // If a different subset of fields was requested, progressively hydrate the record.
            if (partialFields != null && dmo.readFields != null)
            {
               // Choose only the unhydrated fields
               partialFields.andNot(dmo.readFields);
            }
         }
         else
         {
            return dmo;
         }

This indicates that the DMO shall be refreshed from the database if more fields are requested.

Why would we ever hydrate or store state for a CAN-FIND? It should not affect any buffers (that are explicitly referenced by 4GL code) so it should be outside of the caching and hydration implementation.

Originally, we did that. If we did a CAN-FIND, we actually fetched the whole row and saved the DMO in the session cache preemptively. This way, we would avoid a second trip or hydration if we will ever require that record again.
Ultimately, the latency to bring all columns was scary and we leveraged the partialFields implementation to retrieve only the id.

From the whole persistence layer and ORM POVs, it is like a FIND item FIELDS(recid(item)) WHERE ... (pseudo-code) or select recid from item where ... (SQL). We did this to avoid changing the flow of queries that expect to get back Record instances which in most cases are loaded in buffers. The CAN-FIND is implemented using a FindQuery that extends RAQ. Both classes are using methods that return Record (e.g. RAQ.execute or RAQ.executeImpl). So we bolted an onlyId parameter that would fetch an INCOMPLETE Record that has only the id set.

                           // CAN-FIND queries on persistent tables should not load any fields
                           if (onlyId && !buffer.isTemporary())
                           {
                              partialFields = new BitSet();
                           }

About BaseRecord.areAllPropertiesLive, it seems like there is a slight issue.
We have the following test case:

IF CAN-FIND(FIRST item NO-LOCK) THEN DO:
    MESSAGE "NOP".
END.

FOR EACH item NO-LOCK:
    DISPLAY item.
END.

After CAN-FIND execution, the item is loaded as INCOMPLETE with no fields. Hydrator is also set to null because in this moment, the needed fields are supposed to be eagerly initialized.
When the first record from FOR EACH item is loaded, it fails on SQLQuery.hydrateRecordImpl > Session.associate.

The culprit in this case is that the DMO comes as INCOMPLETE from Session cache and the BaseRecord.areAllPropertiesLive returns true because hydrator is set to null.

The difference from the last case is that we are not in lazy mode.

Why would we ever hydrate or store state for a CAN-FIND? It should not affect any buffers (that are explicitly referenced by 4GL code) so it should be outside of the caching and hydration implementation.

Originally, we did that. If we did a CAN-FIND, we actually fetched the whole row and saved the DMO in the session cache preemptively. This way, we would avoid a second trip or hydration if we will ever require that record again.
Ultimately, the latency to bring all columns was scary and we leveraged the partialFields implementation to retrieve only the id.

Fetching the entire row for a CAN-FIND is the opposite of what I'm suggesting. Instead of using CAN-FIND as an opportunity to cache, I'm saying we should NOT ADD EXTRA WORK where OE compatability doesn't require it. CAN-FIND should NEVER bring back a record and should NEVER affect application-level buffers and should NEVER be hydrated, incomplete or otherwise.

Make CAN-FIND fast and stateless. Disconnect it from the rest of the stateful persistence framework.

This same issue has been my concern with the approach on this task all along. From the beginning we have been storing more state than needed. The point of this task is to minimize the state we store to only that which is absolutely needed for compatability purposes. I don't think we gotten there yet.

Alexandru, I have analyzed the server-side memory consumption and the system memory consumption.
I see a about the same memory consumption system-wide.
On FWD server, the memory has decreased, but we keep these result set open which makes PSQL to store result copy on database-side.

On FWD server, the memory has decreased, but we keep these result set open which makes PSQL to store result copy on database-side.

This is the core principle here. A record would not be hydrated, but it would store a weak reference for the result-set that might hydrate it when needed. The data should reside somewhere. With #6720, it resides in the rs. If the query that strongly retains the rs is closed, then the rs is gone. In this case, the record should be fetched from the database again.

#6720 is a performance improvement, not a memory improvement. It skips hydration of fields that are not going to be used.

Make CAN-FIND fast and stateless. Disconnect it from the rest of the stateful persistence framework.

We can do that. But, currently, CAN-FIND is going through RAQ.execute that does several things. Disconnecting it will mean duplicating most of execute work.

• fast-find cache integration: CAN-FIND is looking up in FFC and if it find a record there, it will return true without DB lookup. Also, if CAN-FIND misses FFC, it will update it when coming back from the DB. This part is non-stateless.
• CAN-FIND(buf where recid(buf) = r) is short-circuited.
• if the WHERE clause is done on a (unique) index of a _temp table, then it is routed directly into H2 direct-access.
• record-nursery and dirty-share are honored before look-up

• return Object instead of Record to allow Long/null to be returned for CAN-FIND. However, there was a painful type checking refactoring that should have been done just to support this. It was too invasive and fragile, allowing execute to return arbitrary objects (e.g. Long): Persistence.load, RAQ.executeImpl, RAQ.execute
• he also suggested instantiating a Record out-of-the-blue, just to fulfill the contract of execute. This would eventually reach CAN-FIND and simply do a null-check. I was afraid of this approach as execute presumes that records session cache or from dirty are returned. It looked dangerous to me to spawn random records. The fragility could make the dummy record reach a buffer by mistake.
• so we conveyed to respect the contract and invariants of execute to return session cached records. This is how CAN-FIND gathers a CACHED INCOMPLETE record with only its id set.

I guess the extraction of CAN-FIND would need a more fundamental refactoring to route RAQ toward Persistence.list instead that returns ids and refactor RAQ to return Object instead of Record.

The point of this task is to minimize the state we store to only that which is absolutely needed for compatability purposes. I don't think we gotten there yet.

The JDBC result-set stores all columns anyway; this is something we can't avoid with the technique in #6720. So, 6720c managed just to skip hydrating columns if not going to be needed.

Initially, it was about FOR EACH blocks that define a reasonable scope for the records loaded in buffers, but the implementation is currently covering FIND as well AFAIK. When the record is evicted from the session cache, its underlying result-set is also closed. This had some downsides:

• records that had a hydrator from a FOR EACH became incomplete after a NEXT. Reusing that record somewhere (e.g. after for each, desiring columns that were not hydrated) would mean reloading it from the database again. So:

for each book share-lock: message book.title. end.
for each book share-lock: message book.author. end.

Is going to execute |book| + |book| queries with 6720c, whereas in trunk it will execute |book| queries, considering that there are less books than the session cache (defaults to 1024). From the statistics gathered in #6720-109, this was not the common case (4M fields lazily hydrated vs 30k fields loaded from database).

• records that had a hydrator from FIND are retaining the prepared statement as well. So if another similar FIND is executed, the prepared statement needs to be re-executed and the rs is closed. This makes some records to loose their hydrators when other FINDs are executed. However, I think this is largely mitigated by FFC, but considering FFC is cross-session, then this probability of loosing hydrators scales with the number of users.

It seems that it behaves about the same.
I have made these measurements because I need to know if fixing the performance findings, but I think I shall find more scenarios which 6720b slows down.

Teodor,
I had a quick look at the patch you posted in #6720-159. What I immediately noticed is that it rolls back Razvan's 16570 revision: "Migrate RECID from 32-bit to 64-bit values.", refs: #9977. Possible other revisions as well.

I have made these measurements because I need to know if fixing the performance findings, but I think I shall find more scenarios which 6720b slows down.

I wonder if we have minimized the hydration needed? My understanding is that previous attempts at this task were over-aggressive in hydrating fields that were not directly needed by the 4GL code.

Ovidiu Maxiniuc wrote:

Teodor,
I had a quick look at the patch you posted in #6720-159. What I immediately noticed is that it rolls back Razvan's 16570 revision: "Migrate RECID from 32-bit to 64-bit values.", refs: #9977. Possible other revisions as well.

I have noticed that after I have put the patch here. I have one patch without #9977 if is needed.

6720b is an old branch, which is hard to rebase to the latest trunk revision and that is the reason why I am using patches for now.

Greg Shah wrote:

I have made these measurements because I need to know if fixing the performance findings, but I think I shall find more scenarios which 6720b slows down.

I wonder if we have minimized the hydration needed? My understanding is that previous attempts at this task were over-aggressive in hydrating fields that were not directly needed by the 4GL code.

I am thinking of doing some kind of analysis during conversion/runtime, which checks the number of used fields (especially the case when the procedure "owns" the buffer scope, eg. buffer scope level is 1).
With this analysis, we can decide if we should do lazy hydration or not.
During runtime, this can be done using bytecode analysis for that dmo interface and during conversion, analyzing the AST tree.

• FIND queries -> SQLQuery.uniqueResults.
• FOR EACH queries (in case of AdaptiveQuery, it's decided by the logic from ProgressiveResults) -> ScrollableResults
• DirectAccessHelper.interpretResponse

Greg Shah wrote:

I wonder if we have minimized the hydration needed? My understanding is that previous attempts at this task were over-aggressive in hydrating fields that were not directly needed by the 4GL code.

One interesting idea which I have found and I think it is the most general solution is Runtime feedback per call-site.

• naively start with lazy hydration.
• instrument getDatum with WithLZS counters, which will be used to determine the number of fields which are being used (in percentage).
• on the next query execution, it will decide which path follows based on the counter stats (lazy or eager).

• handles dynamic/escape automatically (it counts real reads, not predicted ones);
• costs a couple of counters per call-site, no ASM, no walking caller bytecode;
• self-tunes per workload.

• a short warm-up window
• a stable per-call-site key and a LRU cache (or a normal cache) which stores these stats.

Until now, the lazy hydration is done just for three things:

We should expand this much further as a first step. After that, we can look at optimizations to minimize cases where the performance is impacted negatively.

Greg Shah wrote:

Until now, the lazy hydration is done just for three things:

We should expand this much further as a first step. After that, we can look at optimizations to minimize cases where the performance is impacted negatively.

I will analyze more cases which we can expand.
One case which might be useful is on Persistence.list (which covers the case for AdaptiveQuery when iterating through the first bracket).

Greg Shah wrote:

Until now, the lazy hydration is done just for three things:

We should expand this much further as a first step. After that, we can look at optimizations to minimize cases where the performance is impacted negatively.

Alex, what is your opinion?

There are two path remaining, Persistence.list and Loader.load, which I think the lazy hydration is not the best solution.

For Persistence.list, it returns a list of Object[], which are not instances of Record, just plain field values from JDBC.
For Loader.load, uses a PK query which always returns 1 result. This will lead to lots of ResultSet opened, which will quickly fill the RESULT_SET_CACHE_SIZE.

From my experiments with runtime feedback approach on customer projects, it seems that the performance is about the same with trunk, with some slight performance improvements in some isolated cases (large cross-joins).

We should expand this much further as a first step. After that, we can look at optimizations to minimize cases where the performance is impacted negatively.

"the lazy hydration is done just for three things: " I wouldn't think this as of a "just three". These cover the vast majority: FOR EACH queries and FIND queries. The list doesn't include PRESELECT basically. As you noted, this is the list API in persistence. Also, Loader.load is an internal mechanism to load a DMO based on its id that was eventually stored in the session cache, but its data went stale. These 2 cases represent (maybe) a very few cases of persistence usage.

• PRESELECT is not reasonable to have lazy loading just because PRESELECT should snapshot the database state in that case. Loading a DMO from the database because its lazy hydrator went stale seems irresponsible.
• Loader.load is an API that is called quite arbitrarily and not always driven deterministically by the converted code. This is used to refresh DMOs that may have been changed by other sessions or using when the information about a DMO (from ffcache or scrolling cache) came back with an id, but the DMO is not in the session cache. It doesn't have a clear lifecycle.

I agree with Teodor that these two cases are not reasonable to lazy hydration.

I'm not sure I agree on the PRESELECT. I'd like to know what the real cases are where we have to load the DMO after it has gone out of scope. No one has shown 4GL code that demonstrates that requirement. If we have the requirement, I think we have problems in our implementation rather than a real requirement from the 4GL.

This is what I mean when I say we are being over-aggressive in hydrating.

Is the current implementation handling things like OPEN QUERY/GET, all scenarios that use things like CompoundQuery and all the cases of dynamic queries?

Greg Shah wrote:

Is the current implementation handling things like OPEN QUERY/GET, all scenarios that use things like CompoundQuery and all the cases of dynamic queries?

Yes.

I'm not sure I agree on the PRESELECT. I'd like to know what the real cases are where we have to load the DMO after it has gone out of scope. No one has shown 4GL code that demonstrates that requirement. If we have the requirement, I think we have problems in our implementation rather than a real requirement from the 4GL.

A DMO doesn't necessarily have a scope considering the Session.cache. The DMOs that are lazily hydrated end up in the session cache. They may stay there even if there is no buffer referencing them.

• If a FIND is executed and the FFC is used, then the id provided. The id is checked against the session cache. The session cache returns the CACHED DMO, but that is lazily hydrated. If a field is queried that was not hydrated and the hydrator is no longer available, then the DMO is re-fetched.
• Same example applies to any cache that gets us an id (e.g. scrolling cache, etc.).

The fundamental difference between trunk and #6720 is that the session cached DMOs may get stale out of the blue if their underlying hydrators are no longer available. In trunk, a cached DMO is (almost) always ready to be used.

Example:

for each tt: message tt.f1. end.
for each tt: message tt.f2. end.

In trunk, there are log(|tt|) queries to the database (considering that for each is progressive). With #6720, there are 2 * log(|tt|) queries, because the session cache populated by the first for each is no longer usable by the second for each. Thus, we have to "load the DMO after it goes out of scope"

Alexandru Lungu wrote:

The fundamental difference between trunk and #6720 is that the session cached DMOs may get stale out of the blue if their underlying hydrators are no longer available. In trunk, a cached DMO is (almost) always ready to be used.

Example:

[...]

In trunk, there are log(|tt|) queries to the database (considering that for each is progressive). With #6720, there are 2 * log(|tt|) queries, because the session cache populated by the first for each is no longer usable by the second for each. Thus, we have to "load the DMO after it goes out of scope"

Ok, so this is due to Session.unownResultSet?

I don't quite fully understand why we should this on #6720, since the ResultSet reflects that image, on the time when the query was made and it is still available.

For the case when a COMMIT happens, or when the connection gets interrupted, you are right.

If a FIND is executed and the FFC is used, then the id provided. The id is checked against the session cache. The session cache returns the CACHED DMO, but that is lazily hydrated. If a field is queried that was not hydrated and the hydrator is no longer available, then the DMO is re-fetched.

This would be outside of the PRESELECT. If the buffers used for a PRESELECT query can't be accessed outside of their scope (which is how it works in OE), then it is completely safe to implement lazy hydration for PRESELECT.

Also, this 4GL example isn't using PRESELECT so I don't see how it justifies leaving PRESELECT out. Don't artificially limit what we can do with this technique.

This would be outside of the PRESELECT. If the buffers used for a PRESELECT query can't be accessed outside of their scope (which is how it works in OE), then it is completely safe to implement lazy hydration for PRESELECT.
Also, this 4GL example isn't using PRESELECT so I don't see how it justifies leaving PRESELECT out. Don't artificially limit what we can do with this technique.

Sorry, I was too focused on the "I'd like to know what the real cases are where we have to load the DMO after it has gone out of scope" statement. If I am not mistaken, the PRESELECT uses list and does the following (or similar):

while (resultSet.next())
            {
               ret.add((T) hydrateRecord(resultSet, rowStruct, 1, session));
            }

This means that only the last record would still have a valid hydrator (a reference to the result-set) by the time the PRESELECT ends. This is because we hydrate all records on PRESELECT query eagerly. To overcome this, we would need to switch to scroll instead of list I guess. I am not sure at this time what implications this might have. Quick impressions:

• the prepared statement will be not reusable, so nested queries using the same SQL would have to rebuild the query
• COMMITS will need to cache the result-set.
• Result-set memory would be kept in memory for a longer time.

Ok, so this is due to Session.unownResultSet?
I don't quite fully understand why we should this on #6720, since the ResultSet reflects that image, on the time when the query was made and it is still available.

Well, if you have a result-set and do 3 next operations, the first two rows are lost. If you did not fully hydrate them in time, then you have to reemit a loading query. At a certain moment, only one single record can be lazily hydrated by a certain result-set. Once that result-set had a next/prev called, the lazy hydration bets are completely off. This is the original design and beg my pardon if this changed in the mean-time. I recall we had a discussion to identify whether we can "get back" the result-set on the right spot in order to continue hydration, but I don't recall ever managing to implement that. I am not 100% sure it is possible:

• considering that a record only has a rs reference, the position shall be inferred (current-position - result-set position) and apply a number of next/previous to match that. I can imagine this being a performance sucker.
• forward-only queries are not susceptible to such thing, unfortunately.
• II am most certain that ProgressiveResults are closing the statement once getting past a bracket. This means that results from a previous bracket (of size 1, 10, 100, etc.) are no longer hydratable.

Again, let's back up here and understand what is the case for 4GL code to access to a specific row's results after it goes out of scope?

In any looping access to a preselect query in the 4GL, the current buffer only ever points to one record. With things like DO PRESELECT or REPEAT PRESELECT, this navigation is tightly coupled to the block iteration. These cases only navigate in one direction and won't ever see previous records again.

With OPEN QUERY PRESELECT the navigation is linked to the GET but in the common case the result is the same. Once you move to the next record you cannot access the previously visible record unless you navigate back to it. Even in code that navigates back and forth, would in most (if not all) cases reference the same fields over and over. If we find real 4GL code that moves around a PRESELECT result set and accesses different fields from time to time, then we can consider other things we can do here like calculating a maximal FIELDS clause that can drive the proper level of hydration at the first access. The only obvious mapping to this back and forth model would be BROWSE but I don't know if PRESELECT can be used with BROWSE. And BROWSE has a fixed set of columns so there won't be a need to re-hydrate.

Alexandru Lungu wrote:

Well, if you have a result-set and do 3 next operations, the first two rows are lost. If you did not fully hydrate them in time, then you have to reemit a loading query. At a certain moment, only one single record can be lazily hydrated by a certain result-set. Once that result-set had a next/prev called, the lazy hydration bets are completely off. This is the original design and beg my pardon if this changed in the mean-time. I recall we had a discussion to identify whether we can "get back" the result-set on the right spot in order to continue hydration, but I don't recall ever managing to implement that. I am not 100% sure it is possible:

Yes, I understand, but this doesn't happen when you have two FOR BLOCKS. The iteration is only FORWARD and you need to go backwards since you will open a new result set on the second FOR.

The problem is relevant when we have a query and we use REPOSITION. I don't know in details yet how we handle REPOSITION, but if we emit new queries, we can replace the hydrators which becomes invalid with the new one.

I think #10554 fix relevant here because there are more Session.associate calls.

There is a bug with this testcase:

// wide6720 has 10000 records
def var s as int64 no-undo.
do transaction:
    for each wide6720 no-lock:
    end.

    for each wide6720 fields(uB) no-lock:
        s = s + wide6720.uB.
    end.
end.

StacktraceStacktrace

java.lang.RuntimeException: invoke() of class com.goldencode.testcases.tests._6720.Test4 and  method execute failed
    at com.goldencode.p2j.util.ControlFlowOps.invokeError(ControlFlowOps.java:8570)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6753)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6522)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1376)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:963)
    at com.goldencode.p2j.main.StandardServer$LegacyInvoker.execute(StandardServer.java:2667)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2056)
    at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:701)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:569)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:126)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:808)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:418)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:237)
    at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:569)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9795)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9752)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6664)
    ... 15 more
Caused by: com.goldencode.p2j.persist.orm.LazyHydrationException: com.goldencode.p2j.persist.PersistenceException: Can't hydrate property that was not selected into the result-set.
Caused by: com.goldencode.p2j.persist.PersistenceException: Can't hydrate property that was not selected into the result-set.

That table has a unique index, composing on (uA, uB).
Since uA is not in the query, because of fields(uB), it returns this error.

Alexandru/Greg, should I rebase 6720b to latest trunk revision or should I port these to a new branch?

I have some changes, which I have not committed yet.

Alexandru/Greg, should I rebase 6720b to latest trunk revision or should I port these to a new branch?

Please rebase. Porting to a new branch will lose history entries and progress on this task. This would be hard to trace back any issues with its corresponding date in time, commit message, author, etc.

In any looping access to a preselect query in the 4GL, the current buffer only ever points to one record. With things like DO PRESELECT or REPEAT PRESELECT, this navigation is tightly coupled to the block iteration. These cases only navigate in one direction and won't ever see previous records again.

I misunderstood the premises of the discussion. I reckon that PRESELECT is lazily hydrated already in #6720 just because it uses .scroll persistence API, which in turn is lazily hydrating. The lazy hydration is not bound to the query type, but the Persistence API (scroll, unqiueResult). When I read Teodor's comment about Persistence.list, I incorrectly bound it to PreselectQuery, but PreselectQuery doesn't call list, but calls scroll. Indeed, list is used for first bracket of FOR EACH ... maybe other usages, Teodor? If it is for the first bracket of a ProgressiveResults, then it is fine to lazily hydrate list.

Just to clarify, it is OK to use it with PRESELECT and AFAIK it is in use. I was under the wrong presumption that .list was being used for PRESELECT query, in which case a switch to .scroll was required and I was mostly uncertain about the side-effects. Apparently, it was using .scroll already.

Ok, I am starting to rebase this branch.

I have tried to reproduce what you told in previous discussions, two FOR blocks and in that second block, emits additional queries because of invalidated hydrator.
I don't think it is possible such case because the result set is being managed by ScrollableResults, which opens and closes the result set, creates the new hydrator objects with the new result set, etc. This case is being handled.

I have found one case where Hydrator.refresh:

def var s as int.
def buffer buf1 for wide6720.
for each wide6720 no-lock:
end.

do num = 9500 to 10000:
   find first buf1 where buf1.uA = num.
   s = buf1.uB.
end.

That extra query does not happen with trunk since it comes eager hydrated from the for each query.

Rebased 6720b to trunk rev 16603. Latest revision is 16632.
A copy of old 6720b is in devsrv01:/tmp/tg.20260616/6720b_old.zip

Right now, I am doing some tests to see if everything is working, and afterwards, I will apply the changes which I have done in this task.

• Implemented runtime feedback field usage feature which decides if it should lazy hydrate or not.

Alexandru, can you take an eye into this change?

I will commit the fix for #6720-182 later on.

Committed with the fix for #6720-179 in 6720b/r16636.