conjure-java icon indicating copy to clipboard operation
conjure-java copied to clipboard

Implement optimization for Primitive lists

Open Shibi-bala opened this issue 1 year ago • 4 comments

Before this PR

Conjure holds primitive arrays as boxed lists. Instead, we can switch to primitive array lists that are backed by primitive typed arrays (double[]) rather than lists (ArrayList<Double>).

After this PR

==COMMIT_MSG== ==COMMIT_MSG==

Possible downsides?

Shibi-bala avatar Apr 01 '24 04:04 Shibi-bala

Generate changelog in changelog/@unreleased

What do the change types mean?
  • feature: A new feature of the service.
  • improvement: An incremental improvement in the functionality or operation of the service.
  • fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
  • break: Has the potential to break consumers of this service's API, inclusive of both Palantir services and external consumers of the service's API (e.g. customer-written software or integrations).
  • deprecation: Advertises the intention to remove service functionality without any change to the operation of the service itself.
  • manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration, performing database surgery, ...) at the time of upgrade for it to succeed.
  • migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?
  • ❗The break and manual task changelog types will result in a major release!
  • 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
  • ✨ All others will result in a minor version release.

Type

  • [ ] Feature
  • [ ] Improvement
  • [ ] Fix
  • [ ] Break
  • [ ] Deprecation
  • [ ] Manual task
  • [ ] Migration

Description Implement optimization for Primitive lists

Check the box to generate changelog(s)

  • [ ] Generate changelog entry

changelog-app[bot] avatar Apr 01 '24 04:04 changelog-app[bot]

In terms of sequencing, I think we should split this up into a few parts:

  1. new generic list factory methods ConjureCollections which do not allow null values + update codegen to use them. note that list<any> should perhaps be allowed to contain null, as List.of(Optional.empty()) serializes to [null] which deserializes as an array containing null.
  2. new specialized list factory methods for primitives in ConjureCollections.java, however these methods return the same types as in [1], but without generic inputs. Codegen can be updated to use these where possible
  3. We can replace the types returned by the primitive-specialized methods with more efficient collection implementations without impacting behavior because we already have validation against nulls.

this sequencing is helpful because it enforces consistent non-nullness for all conjure collections, rather than based on specific types being used.

carterkozak avatar Apr 05 '24 17:04 carterkozak

new generic list factory methods ConjureCollections which do not allow null values + update codegen to use them

@carterkozak Just to clarify this, should these new non-null ConjureCollections methods only be used if options.nonNullCollections is true. Otherwise, this is going to cause a break right?

Shibi-bala avatar Apr 08 '24 15:04 Shibi-bala

That's correct

carterkozak avatar Apr 08 '24 15:04 carterkozak